Skip to content

Commit aa2ba4d

Browse files
tannewttannewt
committed
Have start and end kwargs respect element size
The comment says it is `buffer[start:end]` but it assumed elements were a single byte long. Now it correctly does multibyte elements from array.array. Fixes adafruit#4988
1 parent 75241c4 commit aa2ba4d

6 files changed

Lines changed: 137 additions & 44 deletions

File tree

‎ports/raspberrypi/bindings/rp2pio/StateMachine.c‎

Lines changed: 35 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -396,18 +396,23 @@ STATIC mp_obj_t rp2pio_statemachine_write(size_t n_args, const mp_obj_t *pos_arg
396396

397397
mp_buffer_info_t bufinfo;
398398
mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_READ);
399+
int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
400+
if (stride_in_bytes > 4) {
401+
mp_raise_ValueError(translate("Buffer elements must be 4 bytes long or less"));
402+
}
399403
int32_t start = args[ARG_start].u_int;
400-
size_t length = bufinfo.len;
404+
size_t length = bufinfo.len / stride_in_bytes;
405+
// Normalize in element size units, not bytes.
401406
normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
407+
408+
// Treat start and length in terms of bytes from now on.
409+
start *= stride_in_bytes;
410+
length *= stride_in_bytes;
411+
402412
if (length == 0) {
403413
return mp_const_none;
404414
}
405415

406-
int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
407-
if (stride_in_bytes > 4) {
408-
mp_raise_ValueError(translate("Buffer elements must be 4 bytes long or less"));
409-
}
410-
411416
bool ok = common_hal_rp2pio_statemachine_write(self, ((uint8_t *)bufinfo.buf) + start, length, stride_in_bytes, args[ARG_swap].u_bool);
412417
if (mp_hal_is_interrupted()) {
413418
return mp_const_none;
@@ -603,19 +608,21 @@ STATIC mp_obj_t rp2pio_statemachine_readinto(size_t n_args, const mp_obj_t *pos_
603608

604609
mp_buffer_info_t bufinfo;
605610
mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_WRITE);
611+
int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
612+
if (stride_in_bytes > 4) {
613+
mp_raise_ValueError(translate("Buffer elements must be 4 bytes long or less"));
614+
}
606615
int32_t start = args[ARG_start].u_int;
607-
size_t length = bufinfo.len;
616+
size_t length = bufinfo.len / stride_in_bytes;
608617
normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
609618

619+
// Treat start and length in terms of bytes from now on.
620+
start *= stride_in_bytes;
621+
length *= stride_in_bytes;
610622
if (length == 0) {
611623
return mp_const_none;
612624
}
613625

614-
int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
615-
if (stride_in_bytes > 4) {
616-
mp_raise_ValueError(translate("Buffer elements must be 4 bytes long or less"));
617-
}
618-
619626
bool ok = common_hal_rp2pio_statemachine_readinto(self, ((uint8_t *)bufinfo.buf) + start, length, stride_in_bytes, args[ARG_swap].u_bool);
620627
if (!ok) {
621628
mp_raise_OSError(MP_EIO);
@@ -674,28 +681,32 @@ STATIC mp_obj_t rp2pio_statemachine_write_readinto(size_t n_args, const mp_obj_t
674681

675682
mp_buffer_info_t buf_out_info;
676683
mp_get_buffer_raise(args[ARG_buffer_out].u_obj, &buf_out_info, MP_BUFFER_READ);
684+
int out_stride_in_bytes = mp_binary_get_size('@', buf_out_info.typecode, NULL);
685+
if (out_stride_in_bytes > 4) {
686+
mp_raise_ValueError(translate("Out-buffer elements must be <= 4 bytes long"));
687+
}
677688
int32_t out_start = args[ARG_out_start].u_int;
678-
size_t out_length = buf_out_info.len;
689+
size_t out_length = buf_out_info.len / out_stride_in_bytes;
679690
normalize_buffer_bounds(&out_start, args[ARG_out_end].u_int, &out_length);
680691

681692
mp_buffer_info_t buf_in_info;
682693
mp_get_buffer_raise(args[ARG_buffer_in].u_obj, &buf_in_info, MP_BUFFER_WRITE);
683-
int32_t in_start = args[ARG_in_start].u_int;
684-
size_t in_length = buf_in_info.len;
685-
normalize_buffer_bounds(&in_start, args[ARG_in_end].u_int, &in_length);
686-
687-
if (out_length == 0 && in_length == 0) {
688-
return mp_const_none;
689-
}
690-
691694
int in_stride_in_bytes = mp_binary_get_size('@', buf_in_info.typecode, NULL);
692695
if (in_stride_in_bytes > 4) {
693696
mp_raise_ValueError(translate("In-buffer elements must be <= 4 bytes long"));
694697
}
698+
int32_t in_start = args[ARG_in_start].u_int;
699+
size_t in_length = buf_in_info.len / in_stride_in_bytes;
700+
normalize_buffer_bounds(&in_start, args[ARG_in_end].u_int, &in_length);
695701

696-
int out_stride_in_bytes = mp_binary_get_size('@', buf_out_info.typecode, NULL);
697-
if (out_stride_in_bytes > 4) {
698-
mp_raise_ValueError(translate("Out-buffer elements must be <= 4 bytes long"));
702+
// Treat start and length in terms of bytes from now on.
703+
out_start *= out_stride_in_bytes;
704+
out_length *= out_stride_in_bytes;
705+
in_start *= in_stride_in_bytes;
706+
in_length *= in_stride_in_bytes;
707+
708+
if (out_length == 0 && in_length == 0) {
709+
return mp_const_none;
699710
}
700711

701712
bool ok = common_hal_rp2pio_statemachine_write_readinto(self,

‎ports/raspberrypi/bindings/rp2pio/StateMachine.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ void common_hal_rp2pio_statemachine_restart(rp2pio_statemachine_obj_t *self);
6464
void common_hal_rp2pio_statemachine_stop(rp2pio_statemachine_obj_t *self);
6565
void common_hal_rp2pio_statemachine_run(rp2pio_statemachine_obj_t *self, const uint16_t *instructions, size_t len);
6666

67-
// Writes out the given data.
67+
// Lengths are in bytes.
6868
bool common_hal_rp2pio_statemachine_write(rp2pio_statemachine_obj_t *self, const uint8_t *data, size_t len, uint8_t stride_in_bytes, bool swap);
6969
bool common_hal_rp2pio_statemachine_background_write(rp2pio_statemachine_obj_t *self, const sm_buf_info *once_obj, const sm_buf_info *loop_obj, uint8_t stride_in_bytes, bool swap);
7070
bool common_hal_rp2pio_statemachine_stop_background_write(rp2pio_statemachine_obj_t *self);

‎shared-bindings/bitbangio/I2C.c‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333

3434
#include "shared/runtime/buffer_helper.h"
3535
#include "shared/runtime/context_manager_helpers.h"
36+
#include "py/binary.h"
3637
#include "py/mperrno.h"
3738
#include "py/runtime.h"
3839
#include "supervisor/shared/translate/translate.h"
@@ -187,10 +188,15 @@ STATIC void readfrom(bitbangio_i2c_obj_t *self, mp_int_t address, mp_obj_t buffe
187188
mp_buffer_info_t bufinfo;
188189
mp_get_buffer_raise(buffer, &bufinfo, MP_BUFFER_WRITE);
189190

190-
size_t length = bufinfo.len;
191+
int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
192+
size_t length = bufinfo.len / stride_in_bytes;
191193
normalize_buffer_bounds(&start, end, &length);
192194
mp_arg_validate_length_min(length, 1, MP_QSTR_buffer);
193195

196+
// Treat start and length in terms of bytes from now on.
197+
start *= stride_in_bytes;
198+
length *= stride_in_bytes;
199+
194200
uint8_t status = shared_module_bitbangio_i2c_read(self, address, ((uint8_t *)bufinfo.buf) + start, length);
195201
if (status != 0) {
196202
mp_raise_OSError(status);
@@ -244,10 +250,15 @@ STATIC void writeto(bitbangio_i2c_obj_t *self, mp_int_t address, mp_obj_t buffer
244250
mp_buffer_info_t bufinfo;
245251
mp_get_buffer_raise(buffer, &bufinfo, MP_BUFFER_READ);
246252

247-
size_t length = bufinfo.len;
253+
int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
254+
size_t length = bufinfo.len / stride_in_bytes;
248255
normalize_buffer_bounds(&start, end, &length);
249256

250-
// do the transfer
257+
// Treat start and length in terms of bytes from now on.
258+
start *= stride_in_bytes;
259+
length *= stride_in_bytes;
260+
261+
// Do the transfer
251262
uint8_t status = shared_module_bitbangio_i2c_write(self, address,
252263
((uint8_t *)bufinfo.buf) + start, length,
253264
stop);

‎shared-bindings/bitbangio/SPI.c‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535

3636
#include "shared/runtime/buffer_helper.h"
3737
#include "shared/runtime/context_manager_helpers.h"
38+
#include "py/binary.h"
3839
#include "py/mperrno.h"
3940
#include "py/runtime.h"
4041
#include "supervisor/shared/translate/translate.h"
@@ -192,9 +193,19 @@ STATIC mp_obj_t bitbangio_spi_obj_unlock(mp_obj_t self_in) {
192193
}
193194
MP_DEFINE_CONST_FUN_OBJ_1(bitbangio_spi_unlock_obj, bitbangio_spi_obj_unlock);
194195

195-
//| def write(self, buf: ReadableBuffer) -> None:
196+
//| import sys
197+
//| def write(self, buf: ReadableBuffer, *, start: int = 0, end: int = sys.maxsize) -> None:
196198
//| """Write the data contained in ``buf``. Requires the SPI being locked.
197-
//| If the buffer is empty, nothing happens."""
199+
//| If the buffer is empty, nothing happens.
200+
//|
201+
//| If ``start`` or ``end`` is provided, then the buffer will be sliced
202+
//| as if ``buffer[start:end]`` were passed, but without copying the data.
203+
//| The number of bytes written will be the length of ``buffer[start:end]``.
204+
//|
205+
//| :param ReadableBuffer buffer: buffer containing the bytes to write
206+
//| :param int start: beginning of buffer slice
207+
//| :param int end: end of buffer slice; if not specified, use ``len(buffer)``
208+
//| """
198209
//| ...
199210
STATIC mp_obj_t bitbangio_spi_write(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
200211
enum { ARG_buffer, ARG_start, ARG_end };
@@ -211,10 +222,16 @@ STATIC mp_obj_t bitbangio_spi_write(size_t n_args, const mp_obj_t *pos_args, mp_
211222

212223
mp_buffer_info_t bufinfo;
213224
mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_READ);
225+
// Compute bounds in terms of elements, not bytes.
226+
int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
214227
int32_t start = args[ARG_start].u_int;
215-
size_t length = bufinfo.len;
228+
size_t length = bufinfo.len / stride_in_bytes;
216229
normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
217230

231+
// Treat start and length in terms of bytes from now on.
232+
start *= stride_in_bytes;
233+
length *= stride_in_bytes;
234+
218235
if (length == 0) {
219236
return mp_const_none;
220237
}
@@ -267,10 +284,16 @@ STATIC mp_obj_t bitbangio_spi_readinto(size_t n_args, const mp_obj_t *pos_args,
267284

268285
mp_buffer_info_t bufinfo;
269286
mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_WRITE);
287+
int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
288+
// Compute bounds in terms of elements, not bytes.
270289
int32_t start = args[ARG_start].u_int;
271-
size_t length = bufinfo.len;
290+
size_t length = bufinfo.len / stride_in_bytes;
272291
normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
273292

293+
// Treat start and length in terms of bytes from now on.
294+
start *= stride_in_bytes;
295+
length *= stride_in_bytes;
296+
274297
if (length == 0) {
275298
return mp_const_none;
276299
}
@@ -337,16 +360,24 @@ STATIC mp_obj_t bitbangio_spi_write_readinto(size_t n_args, const mp_obj_t *pos_
337360

338361
mp_buffer_info_t buf_out_info;
339362
mp_get_buffer_raise(args[ARG_out_buffer].u_obj, &buf_out_info, MP_BUFFER_READ);
363+
int out_stride_in_bytes = mp_binary_get_size('@', buf_out_info.typecode, NULL);
340364
int32_t out_start = args[ARG_out_start].u_int;
341-
size_t out_length = buf_out_info.len;
365+
size_t out_length = buf_out_info.len / out_stride_in_bytes;
342366
normalize_buffer_bounds(&out_start, args[ARG_out_end].u_int, &out_length);
343367

344368
mp_buffer_info_t buf_in_info;
345369
mp_get_buffer_raise(args[ARG_in_buffer].u_obj, &buf_in_info, MP_BUFFER_WRITE);
370+
int in_stride_in_bytes = mp_binary_get_size('@', buf_in_info.typecode, NULL);
346371
int32_t in_start = args[ARG_in_start].u_int;
347-
size_t in_length = buf_in_info.len;
372+
size_t in_length = buf_in_info.len / in_stride_in_bytes;
348373
normalize_buffer_bounds(&in_start, args[ARG_in_end].u_int, &in_length);
349374

375+
// Treat start and length in terms of bytes from now on.
376+
out_start *= out_stride_in_bytes;
377+
out_length *= out_stride_in_bytes;
378+
in_start *= in_stride_in_bytes;
379+
in_length *= in_stride_in_bytes;
380+
350381
if (out_length != in_length) {
351382
mp_raise_ValueError(translate("buffer slices must be of equal length"));
352383
}

‎shared-bindings/busio/I2C.c‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333

3434
#include "shared/runtime/buffer_helper.h"
3535
#include "shared/runtime/context_manager_helpers.h"
36+
#include "py/binary.h"
3637
#include "py/runtime.h"
3738
#include "supervisor/shared/translate/translate.h"
3839

@@ -207,12 +208,18 @@ STATIC mp_obj_t busio_i2c_readfrom_into(size_t n_args, const mp_obj_t *pos_args,
207208
mp_buffer_info_t bufinfo;
208209
mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_WRITE);
209210

210-
size_t length = bufinfo.len;
211+
// Compute bounds in terms of elements, not bytes.
212+
int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
213+
size_t length = bufinfo.len / stride_in_bytes;
211214
int32_t start = args[ARG_start].u_int;
212215
const int32_t end = args[ARG_end].u_int;
213216
normalize_buffer_bounds(&start, end, &length);
214217
mp_arg_validate_length_min(length, 1, MP_QSTR_buffer);
215218

219+
// Treat start and length in terms of bytes from now on.
220+
start *= stride_in_bytes;
221+
length *= stride_in_bytes;
222+
216223
uint8_t status =
217224
common_hal_busio_i2c_read(self, args[ARG_address].u_int, ((uint8_t *)bufinfo.buf) + start, length);
218225
if (status != 0) {
@@ -260,12 +267,18 @@ STATIC mp_obj_t busio_i2c_writeto(size_t n_args, const mp_obj_t *pos_args, mp_ma
260267
// get the buffer to write the data from
261268
mp_buffer_info_t bufinfo;
262269
mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_READ);
270+
int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
263271

264-
size_t length = bufinfo.len;
272+
// Compute bounds in terms of elements, not bytes.
273+
size_t length = bufinfo.len / stride_in_bytes;
265274
int32_t start = args[ARG_start].u_int;
266275
const int32_t end = args[ARG_end].u_int;
267276
normalize_buffer_bounds(&start, end, &length);
268277

278+
// Treat start and length in terms of bytes from now on.
279+
start *= stride_in_bytes;
280+
length *= stride_in_bytes;
281+
269282
// do the transfer
270283
uint8_t status =
271284
common_hal_busio_i2c_write(self, args[ARG_address].u_int, ((uint8_t *)bufinfo.buf) + start, length);
@@ -331,23 +344,29 @@ STATIC mp_obj_t busio_i2c_writeto_then_readfrom(size_t n_args, const mp_obj_t *p
331344

332345
mp_buffer_info_t out_bufinfo;
333346
mp_get_buffer_raise(args[ARG_out_buffer].u_obj, &out_bufinfo, MP_BUFFER_READ);
334-
335-
size_t out_length = out_bufinfo.len;
347+
int out_stride_in_bytes = mp_binary_get_size('@', out_bufinfo.typecode, NULL);
348+
size_t out_length = out_bufinfo.len / out_stride_in_bytes;
336349
int32_t out_start = args[ARG_out_start].u_int;
337350
const int32_t out_end = args[ARG_out_end].u_int;
338351
normalize_buffer_bounds(&out_start, out_end, &out_length);
339352

340353
mp_buffer_info_t in_bufinfo;
341354
mp_get_buffer_raise(args[ARG_in_buffer].u_obj, &in_bufinfo, MP_BUFFER_WRITE);
342-
343-
size_t in_length = in_bufinfo.len;
355+
int in_stride_in_bytes = mp_binary_get_size('@', in_bufinfo.typecode, NULL);
356+
size_t in_length = in_bufinfo.len / in_stride_in_bytes;
344357
int32_t in_start = args[ARG_in_start].u_int;
345358
const int32_t in_end = args[ARG_in_end].u_int;
346359
normalize_buffer_bounds(&in_start, in_end, &in_length);
347360
mp_arg_validate_length_min(in_length, 1, MP_QSTR_out_buffer);
348361

362+
// Treat start and length in terms of bytes from now on.
363+
out_start *= out_stride_in_bytes;
364+
out_length *= out_stride_in_bytes;
365+
in_start *= in_stride_in_bytes;
366+
in_length *= in_stride_in_bytes;
367+
349368
uint8_t status = common_hal_busio_i2c_write_read(self, args[ARG_address].u_int,
350-
((uint8_t *)out_bufinfo.buf) + out_start, out_length,((uint8_t *)in_bufinfo.buf) + in_start, in_length);
369+
((uint8_t *)out_bufinfo.buf) + out_start, out_length, ((uint8_t *)in_bufinfo.buf) + in_start, in_length);
351370
if (status != 0) {
352371
mp_raise_OSError(status);
353372
}

0 commit comments

Comments
 (0)