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 #4988

Author: tannewt

ports/raspberrypi/bindings/rp2pio/StateMachine.c

@@ -396,18 +396,23 @@ STATIC mp_obj_t rp2pio_statemachine_write(size_t n_args, const mp_obj_t *pos_arg
 
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_READ);
+    int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
+    if (stride_in_bytes > 4) {
+        mp_raise_ValueError(translate("Buffer elements must be 4 bytes long or less"));
+    }
     int32_t start = args[ARG_start].u_int;
-    size_t length = bufinfo.len;
+    size_t length = bufinfo.len / stride_in_bytes;
+    // Normalize in element size units, not bytes.
     normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
+
+    // Treat start and length in terms of bytes from now on.
+    start *= stride_in_bytes;
+    length *= stride_in_bytes;
+
     if (length == 0) {
         return mp_const_none;
     }
 
-    int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
-    if (stride_in_bytes > 4) {
-        mp_raise_ValueError(translate("Buffer elements must be 4 bytes long or less"));
-    }
-
     bool ok = common_hal_rp2pio_statemachine_write(self, ((uint8_t *)bufinfo.buf) + start, length, stride_in_bytes, args[ARG_swap].u_bool);
     if (mp_hal_is_interrupted()) {
         return mp_const_none;
@@ -603,19 +608,21 @@ STATIC mp_obj_t rp2pio_statemachine_readinto(size_t n_args, const mp_obj_t *pos_
 
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_WRITE);
+    int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
+    if (stride_in_bytes > 4) {
+        mp_raise_ValueError(translate("Buffer elements must be 4 bytes long or less"));
+    }
     int32_t start = args[ARG_start].u_int;
-    size_t length = bufinfo.len;
+    size_t length = bufinfo.len / stride_in_bytes;
     normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
 
+    // Treat start and length in terms of bytes from now on.
+    start *= stride_in_bytes;
+    length *= stride_in_bytes;
     if (length == 0) {
         return mp_const_none;
     }
 
-    int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
-    if (stride_in_bytes > 4) {
-        mp_raise_ValueError(translate("Buffer elements must be 4 bytes long or less"));
-    }
-
     bool ok = common_hal_rp2pio_statemachine_readinto(self, ((uint8_t *)bufinfo.buf) + start, length, stride_in_bytes, args[ARG_swap].u_bool);
     if (!ok) {
         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
 
     mp_buffer_info_t buf_out_info;
     mp_get_buffer_raise(args[ARG_buffer_out].u_obj, &buf_out_info, MP_BUFFER_READ);
+    int out_stride_in_bytes = mp_binary_get_size('@', buf_out_info.typecode, NULL);
+    if (out_stride_in_bytes > 4) {
+        mp_raise_ValueError(translate("Out-buffer elements must be <= 4 bytes long"));
+    }
     int32_t out_start = args[ARG_out_start].u_int;
-    size_t out_length = buf_out_info.len;
+    size_t out_length = buf_out_info.len / out_stride_in_bytes;
     normalize_buffer_bounds(&out_start, args[ARG_out_end].u_int, &out_length);
 
     mp_buffer_info_t buf_in_info;
     mp_get_buffer_raise(args[ARG_buffer_in].u_obj, &buf_in_info, MP_BUFFER_WRITE);
-    int32_t in_start = args[ARG_in_start].u_int;
-    size_t in_length = buf_in_info.len;
-    normalize_buffer_bounds(&in_start, args[ARG_in_end].u_int, &in_length);
-
-    if (out_length == 0 && in_length == 0) {
-        return mp_const_none;
-    }
-
     int in_stride_in_bytes = mp_binary_get_size('@', buf_in_info.typecode, NULL);
     if (in_stride_in_bytes > 4) {
         mp_raise_ValueError(translate("In-buffer elements must be <= 4 bytes long"));
     }
+    int32_t in_start = args[ARG_in_start].u_int;
+    size_t in_length = buf_in_info.len / in_stride_in_bytes;
+    normalize_buffer_bounds(&in_start, args[ARG_in_end].u_int, &in_length);
 
-    int out_stride_in_bytes = mp_binary_get_size('@', buf_out_info.typecode, NULL);
-    if (out_stride_in_bytes > 4) {
-        mp_raise_ValueError(translate("Out-buffer elements must be <= 4 bytes long"));
+    // Treat start and length in terms of bytes from now on.
+    out_start *= out_stride_in_bytes;
+    out_length *= out_stride_in_bytes;
+    in_start *= in_stride_in_bytes;
+    in_length *= in_stride_in_bytes;
+
+    if (out_length == 0 && in_length == 0) {
+        return mp_const_none;
     }
 
     bool ok = common_hal_rp2pio_statemachine_write_readinto(self,

ports/raspberrypi/bindings/rp2pio/StateMachine.h

@@ -64,7 +64,7 @@ void common_hal_rp2pio_statemachine_restart(rp2pio_statemachine_obj_t *self);
 void common_hal_rp2pio_statemachine_stop(rp2pio_statemachine_obj_t *self);
 void common_hal_rp2pio_statemachine_run(rp2pio_statemachine_obj_t *self, const uint16_t *instructions, size_t len);
 
-// Writes out the given data.
+// Lengths are in bytes.
 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);
 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);
 bool common_hal_rp2pio_statemachine_stop_background_write(rp2pio_statemachine_obj_t *self);

shared-bindings/bitbangio/I2C.c

@@ -33,6 +33,7 @@
 
 #include "shared/runtime/buffer_helper.h"
 #include "shared/runtime/context_manager_helpers.h"
+#include "py/binary.h"
 #include "py/mperrno.h"
 #include "py/runtime.h"
 #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
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(buffer, &bufinfo, MP_BUFFER_WRITE);
 
-    size_t length = bufinfo.len;
+    int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
+    size_t length = bufinfo.len / stride_in_bytes;
     normalize_buffer_bounds(&start, end, &length);
     mp_arg_validate_length_min(length, 1, MP_QSTR_buffer);
 
+    // Treat start and length in terms of bytes from now on.
+    start *= stride_in_bytes;
+    length *= stride_in_bytes;
+
     uint8_t status = shared_module_bitbangio_i2c_read(self, address, ((uint8_t *)bufinfo.buf) + start, length);
     if (status != 0) {
         mp_raise_OSError(status);
@@ -244,10 +250,15 @@ STATIC void writeto(bitbangio_i2c_obj_t *self, mp_int_t address, mp_obj_t buffer
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(buffer, &bufinfo, MP_BUFFER_READ);
 
-    size_t length = bufinfo.len;
+    int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
+    size_t length = bufinfo.len / stride_in_bytes;
     normalize_buffer_bounds(&start, end, &length);
 
-    // do the transfer
+    // Treat start and length in terms of bytes from now on.
+    start *= stride_in_bytes;
+    length *= stride_in_bytes;
+
+    // Do the transfer
     uint8_t status = shared_module_bitbangio_i2c_write(self, address,
         ((uint8_t *)bufinfo.buf) + start, length,
         stop);

shared-bindings/bitbangio/SPI.c

@@ -35,6 +35,7 @@
 
 #include "shared/runtime/buffer_helper.h"
 #include "shared/runtime/context_manager_helpers.h"
+#include "py/binary.h"
 #include "py/mperrno.h"
 #include "py/runtime.h"
 #include "supervisor/shared/translate/translate.h"
@@ -192,9 +193,19 @@ STATIC mp_obj_t bitbangio_spi_obj_unlock(mp_obj_t self_in) {
 }
 MP_DEFINE_CONST_FUN_OBJ_1(bitbangio_spi_unlock_obj, bitbangio_spi_obj_unlock);
 
-//|     def write(self, buf: ReadableBuffer) -> None:
+//|     import sys
+//|     def write(self, buf: ReadableBuffer, *, start: int = 0, end: int = sys.maxsize) -> None:
 //|         """Write the data contained in ``buf``. Requires the SPI being locked.
-//|         If the buffer is empty, nothing happens."""
+//|         If the buffer is empty, nothing happens.
+//|
+//|         If ``start`` or ``end`` is provided, then the buffer will be sliced
+//|         as if ``buffer[start:end]`` were passed, but without copying the data.
+//|         The number of bytes written will be the length of ``buffer[start:end]``.
+//|
+//|         :param ReadableBuffer buffer: buffer containing the bytes to write
+//|         :param int start: beginning of buffer slice
+//|         :param int end: end of buffer slice; if not specified, use ``len(buffer)``
+//|         """
 //|         ...
 STATIC mp_obj_t bitbangio_spi_write(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
     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_
 
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_READ);
+    // Compute bounds in terms of elements, not bytes.
+    int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
     int32_t start = args[ARG_start].u_int;
-    size_t length = bufinfo.len;
+    size_t length = bufinfo.len / stride_in_bytes;
     normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
 
+    // Treat start and length in terms of bytes from now on.
+    start *= stride_in_bytes;
+    length *= stride_in_bytes;
+
     if (length == 0) {
         return mp_const_none;
     }
@@ -267,10 +284,16 @@ STATIC mp_obj_t bitbangio_spi_readinto(size_t n_args, const mp_obj_t *pos_args,
 
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_WRITE);
+    int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
+    // Compute bounds in terms of elements, not bytes.
     int32_t start = args[ARG_start].u_int;
-    size_t length = bufinfo.len;
+    size_t length = bufinfo.len / stride_in_bytes;
     normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
 
+    // Treat start and length in terms of bytes from now on.
+    start *= stride_in_bytes;
+    length *= stride_in_bytes;
+
     if (length == 0) {
         return mp_const_none;
     }
@@ -337,16 +360,24 @@ STATIC mp_obj_t bitbangio_spi_write_readinto(size_t n_args, const mp_obj_t *pos_
 
     mp_buffer_info_t buf_out_info;
     mp_get_buffer_raise(args[ARG_out_buffer].u_obj, &buf_out_info, MP_BUFFER_READ);
+    int out_stride_in_bytes = mp_binary_get_size('@', buf_out_info.typecode, NULL);
     int32_t out_start = args[ARG_out_start].u_int;
-    size_t out_length = buf_out_info.len;
+    size_t out_length = buf_out_info.len / out_stride_in_bytes;
     normalize_buffer_bounds(&out_start, args[ARG_out_end].u_int, &out_length);
 
     mp_buffer_info_t buf_in_info;
     mp_get_buffer_raise(args[ARG_in_buffer].u_obj, &buf_in_info, MP_BUFFER_WRITE);
+    int in_stride_in_bytes = mp_binary_get_size('@', buf_in_info.typecode, NULL);
     int32_t in_start = args[ARG_in_start].u_int;
-    size_t in_length = buf_in_info.len;
+    size_t in_length = buf_in_info.len / in_stride_in_bytes;
     normalize_buffer_bounds(&in_start, args[ARG_in_end].u_int, &in_length);
 
+    // Treat start and length in terms of bytes from now on.
+    out_start *= out_stride_in_bytes;
+    out_length *= out_stride_in_bytes;
+    in_start *= in_stride_in_bytes;
+    in_length *= in_stride_in_bytes;
+
     if (out_length != in_length) {
         mp_raise_ValueError(translate("buffer slices must be of equal length"));
     }

shared-bindings/busio/I2C.c

@@ -33,6 +33,7 @@
 
 #include "shared/runtime/buffer_helper.h"
 #include "shared/runtime/context_manager_helpers.h"
+#include "py/binary.h"
 #include "py/runtime.h"
 #include "supervisor/shared/translate/translate.h"
 
@@ -207,12 +208,18 @@ STATIC mp_obj_t busio_i2c_readfrom_into(size_t n_args, const mp_obj_t *pos_args,
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_WRITE);
 
-    size_t length = bufinfo.len;
+    // Compute bounds in terms of elements, not bytes.
+    int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
+    size_t length = bufinfo.len / stride_in_bytes;
     int32_t start = args[ARG_start].u_int;
     const int32_t end = args[ARG_end].u_int;
     normalize_buffer_bounds(&start, end, &length);
     mp_arg_validate_length_min(length, 1, MP_QSTR_buffer);
 
+    // Treat start and length in terms of bytes from now on.
+    start *= stride_in_bytes;
+    length *= stride_in_bytes;
+
     uint8_t status =
         common_hal_busio_i2c_read(self, args[ARG_address].u_int, ((uint8_t *)bufinfo.buf) + start, length);
     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
     // get the buffer to write the data from
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_READ);
+    int stride_in_bytes = mp_binary_get_size('@', bufinfo.typecode, NULL);
 
-    size_t length = bufinfo.len;
+    // Compute bounds in terms of elements, not bytes.
+    size_t length = bufinfo.len / stride_in_bytes;
     int32_t start = args[ARG_start].u_int;
     const int32_t end = args[ARG_end].u_int;
     normalize_buffer_bounds(&start, end, &length);
 
+    // Treat start and length in terms of bytes from now on.
+    start *= stride_in_bytes;
+    length *= stride_in_bytes;
+
     // do the transfer
     uint8_t status =
         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
 
     mp_buffer_info_t out_bufinfo;
     mp_get_buffer_raise(args[ARG_out_buffer].u_obj, &out_bufinfo, MP_BUFFER_READ);
-
-    size_t out_length = out_bufinfo.len;
+    int out_stride_in_bytes = mp_binary_get_size('@', out_bufinfo.typecode, NULL);
+    size_t out_length = out_bufinfo.len / out_stride_in_bytes;
     int32_t out_start = args[ARG_out_start].u_int;
     const int32_t out_end = args[ARG_out_end].u_int;
     normalize_buffer_bounds(&out_start, out_end, &out_length);
 
     mp_buffer_info_t in_bufinfo;
     mp_get_buffer_raise(args[ARG_in_buffer].u_obj, &in_bufinfo, MP_BUFFER_WRITE);
-
-    size_t in_length = in_bufinfo.len;
+    int in_stride_in_bytes = mp_binary_get_size('@', in_bufinfo.typecode, NULL);
+    size_t in_length = in_bufinfo.len / in_stride_in_bytes;
     int32_t in_start = args[ARG_in_start].u_int;
     const int32_t in_end = args[ARG_in_end].u_int;
     normalize_buffer_bounds(&in_start, in_end, &in_length);
     mp_arg_validate_length_min(in_length, 1, MP_QSTR_out_buffer);
 
+    // Treat start and length in terms of bytes from now on.
+    out_start *= out_stride_in_bytes;
+    out_length *= out_stride_in_bytes;
+    in_start *= in_stride_in_bytes;
+    in_length *= in_stride_in_bytes;
+
     uint8_t status = common_hal_busio_i2c_write_read(self, args[ARG_address].u_int,
-        ((uint8_t *)out_bufinfo.buf) + out_start, out_length,((uint8_t *)in_bufinfo.buf) + in_start, in_length);
+        ((uint8_t *)out_bufinfo.buf) + out_start, out_length, ((uint8_t *)in_bufinfo.buf) + in_start, in_length);
     if (status != 0) {
         mp_raise_OSError(status);
     }