ParallelImageCapture: Switch to taking a list of pins

.. adopting validate_pins from RGBMatrix into shared-bindings

.. updating other platforms for API change

Author: jepler

locale/circuitpython.pot

@@ -1280,6 +1280,11 @@ msgstr ""
 msgid "Invalid data_count %d"
 msgstr ""
 
+#: ports/atmel-samd/common-hal/imagecapture/ParallelImageCapture.c
+#, c-format
+msgid "Invalid data_pins[%d]"
+msgstr ""
+
 #: shared-bindings/digitalio/DigitalInOut.c
 msgid "Invalid direction."
 msgstr ""
@@ -1813,6 +1818,7 @@ msgid ""
 "constructor"
 msgstr ""
 
+#: ports/raspberrypi/common-hal/imagecapture/ParallelImageCapture.c
 #: ports/raspberrypi/common-hal/rotaryio/IncrementalEncoder.c
 msgid "Pins must be sequential"
 msgstr ""

ports/atmel-samd/common-hal/imagecapture/ParallelImageCapture.c

@@ -50,13 +50,16 @@
 #define PIN_PCC_CLK (PIN_PA14)
 
 void common_hal_imagecapture_parallelimagecapture_construct(imagecapture_parallelimagecapture_obj_t *self,
-    const mcu_pin_obj_t *data0,
+    const uint8_t data_pins[],
+    uint8_t data_count,
     const mcu_pin_obj_t *data_clock,
     const mcu_pin_obj_t *vertical_sync,
-    const mcu_pin_obj_t *horizontal_reference,
-    int data_count) {
-    if (data0->number != PIN_PCC_D0) {
-        mp_raise_ValueError_varg(translate("Invalid %q pin"), MP_QSTR_data0);
+    const mcu_pin_obj_t *horizontal_reference) {
+
+    for (int i = 0; i < data_count; i++) {
+        if (data_pins[i] != PIN_PCC_D0 + i) {
+            mp_raise_ValueError_varg(translate("Invalid data_pins[%d]"), i);
+        }
     }
     // The peripheral supports 8, 10, 12, or 14 data bits, but the code only supports 8 at present
     if (data_count != 8) {
@@ -73,7 +76,7 @@ void common_hal_imagecapture_parallelimagecapture_construct(imagecapture_paralle
     }
     // technically, 0 was validated as free already but check again
     for (int i = 0; i < data_count; i++) {
-        if (!pin_number_is_free(data0->number + i)) {
+        if (!pin_number_is_free(data_pins[i])) {
             mp_raise_ValueError_varg(translate("data pin #%d in use"), i);
         }
     }

ports/raspberrypi/common-hal/imagecapture/ParallelImageCapture.c

@@ -68,12 +68,33 @@
         /* .wrap */ \
     }
 
+mcu_pin_obj_t *pin_from_number(uint8_t number) {
+    const mp_map_t *mcu_map = &mcu_pin_globals.map;
+    for (uint8_t i = 0; i < mcu_map->alloc; i++) {
+        mp_obj_t val = mcu_map->table[i].value;
+        if (!mp_obj_is_type(val, &mcu_pin_type)) {
+            continue;
+        }
+        mcu_pin_obj_t *pin = MP_OBJ_TO_PTR(val);
+        if (pin->number == number) {
+            return pin;
+        }
+    }
+    return NULL;
+}
+
 void common_hal_imagecapture_parallelimagecapture_construct(imagecapture_parallelimagecapture_obj_t *self,
-    const mcu_pin_obj_t *data0,
+    const uint8_t data_pins[],
+    uint8_t data_count,
     const mcu_pin_obj_t *data_clock,
     const mcu_pin_obj_t *vertical_sync,
-    const mcu_pin_obj_t *horizontal_reference,
-    int data_count) {
+    const mcu_pin_obj_t *horizontal_reference) {
+
+    for (int i = 1; i < data_count; i++) {
+        if (data_pins[i] - data_pins[0] != i) {
+            mp_raise_RuntimeError(translate("Pins must be sequential"));
+        }
+    }
 
     uint16_t imagecapture_code[] = IMAGECAPTURE_CODE(data_count, data_clock->number, vertical_sync->number, horizontal_reference->number);
 
@@ -82,7 +103,7 @@ void common_hal_imagecapture_parallelimagecapture_construct(imagecapture_paralle
         common_hal_mcu_processor_get_frequency(), // full speed (4 instructions per loop -> max pclk 30MHz @ 120MHz)
         0, 0, // init
         NULL, 0, 0, 0, // out pins
-        data0, data_count, // in pins
+        pin_from_number(data_pins[0]), data_count, // in pins
         0, 0, // in pulls
         NULL, 0, 0, 0, // set pins
         #if DEBUG_STATE_MACHINE

shared-bindings/imagecapture/ParallelImageCapture.c

@@ -39,27 +39,24 @@
 //|     def __init__(
 //|         self,
 //|         *,
-//|         data0: microcontroller.Pin,
-//|         data_count: int=8,
+//|         data_pins: List[microcontroller.Pin],
 //|         clock: microcontroller.Pin,
 //|         vsync: Optional[microcontroller.Pin],
 //|         href: Optional[microcontroller.Pin],
 //|     ):
 //|         """Create a parallel image capture object
 //|         :param microcontroller.Pin data0: The first data pin.  Additional data pins are assumed to follow this pin directly in the microcontroller's standard pin ordering.
-//|         :param int data_count: The number of data pins.
 //|         :param microcontroller.Pin clock: The pixel clock input.
 //|         :param microcontroller.Pin vsync: The vertical sync input, which has a negative-going pulse at the beginning of each frame.
 //|         :param microcontroller.Pin href: The horizontal reference input, which is high whenever the camera is transmitting valid pixel information.
 //|         """
 //|         ...
 //|
 STATIC mp_obj_t imagecapture_parallelimagecapture_make_new(const mp_obj_type_t *type, size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
-    enum { ARG_data0, ARG_data_count, ARG_clock, ARG_vsync, ARG_href,
+    enum { ARG_data_pins, ARG_clock, ARG_vsync, ARG_href,
            NUM_ARGS };
     static const mp_arg_t allowed_args[] = {
-        { MP_QSTR_data0, MP_ARG_OBJ | MP_ARG_REQUIRED | MP_ARG_KW_ONLY },
-        { MP_QSTR_data_count, MP_ARG_INT | MP_ARG_KW_ONLY, { .u_int = 8 } },
+        { MP_QSTR_data_pins, MP_ARG_OBJ | MP_ARG_KW_ONLY, { .u_obj = MP_ROM_NONE } },
         { MP_QSTR_clock, MP_ARG_OBJ | MP_ARG_REQUIRED | MP_ARG_KW_ONLY },
         { MP_QSTR_vsync, MP_ARG_OBJ | MP_ARG_REQUIRED | MP_ARG_KW_ONLY },
         { MP_QSTR_href, MP_ARG_OBJ | MP_ARG_REQUIRED | MP_ARG_KW_ONLY },
@@ -68,15 +65,18 @@ STATIC mp_obj_t imagecapture_parallelimagecapture_make_new(const mp_obj_type_t *
     MP_STATIC_ASSERT(MP_ARRAY_SIZE(allowed_args) == NUM_ARGS);
     mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
 
-    mcu_pin_obj_t *data0 = validate_obj_is_free_pin(args[ARG_data0].u_obj);
+    uint8_t pins[32];
+    uint8_t pin_count;
+    validate_pins(MP_QSTR_data, pins, MP_ARRAY_SIZE(pins), args[ARG_data_pins].u_obj, &pin_count);
+
     mcu_pin_obj_t *clock = validate_obj_is_free_pin(args[ARG_clock].u_obj);
     mcu_pin_obj_t *vsync = validate_obj_is_free_pin_or_none(args[ARG_vsync].u_obj);
     mcu_pin_obj_t *href = validate_obj_is_free_pin_or_none(args[ARG_href].u_obj);
 
     imagecapture_parallelimagecapture_obj_t *self = m_new_obj(imagecapture_parallelimagecapture_obj_t);
     self->base.type = &imagecapture_parallelimagecapture_type;
 
-    common_hal_imagecapture_parallelimagecapture_construct(self, data0, clock, vsync, href, args[ARG_data_count].u_int);
+    common_hal_imagecapture_parallelimagecapture_construct(self, pins, pin_count, clock, vsync, href);
 
     return self;
 }

shared-bindings/imagecapture/ParallelImageCapture.h

@@ -31,12 +31,13 @@
 typedef struct imagecapture_parallelimagecapture_obj imagecapture_parallelimagecapture_obj_t;
 extern const mp_obj_type_t imagecapture_parallelimagecapture_type;
 
+// if only the first element of data_pins is non-NULL, the pins are sequential in microcontroller pin numbering.
 void common_hal_imagecapture_parallelimagecapture_construct(imagecapture_parallelimagecapture_obj_t *self,
-    const mcu_pin_obj_t *data0,
+    const uint8_t data_pins[],
+    uint8_t data_count,
     const mcu_pin_obj_t *data_clock,
     const mcu_pin_obj_t *vertical_sync,
-    const mcu_pin_obj_t *horizontal_sync,
-    int data_count);
+    const mcu_pin_obj_t *horizontal_sync);
 void common_hal_imagecapture_parallelimagecapture_deinit(imagecapture_parallelimagecapture_obj_t *self);
 bool common_hal_imagecapture_parallelimagecapture_deinited(imagecapture_parallelimagecapture_obj_t *self);
 void common_hal_imagecapture_parallelimagecapture_capture(imagecapture_parallelimagecapture_obj_t *self, void *buffer, size_t bufsize);

shared-bindings/microcontroller/Pin.c

@@ -137,3 +137,11 @@ void assert_pin_free(const mcu_pin_obj_t *pin) {
         mp_raise_ValueError_varg(translate("%q in use"), name);
     }
 }
+
+void validate_pins(qstr what, uint8_t *pin_nos, mp_int_t max_pins, mp_obj_t seq, uint8_t *count_out) {
+    mcu_pin_obj_t *pins[max_pins];
+    validate_list_is_free_pins(what, pins, max_pins, seq, count_out);
+    for (mp_int_t i = 0; i < *count_out; i++) {
+        pin_nos[i] = common_hal_mcu_pin_number(pins[i]);
+    }
+}

shared-bindings/microcontroller/Pin.h

@@ -38,6 +38,7 @@ mcu_pin_obj_t *validate_obj_is_pin_or_none(mp_obj_t obj);
 mcu_pin_obj_t *validate_obj_is_free_pin(mp_obj_t obj);
 mcu_pin_obj_t *validate_obj_is_free_pin_or_none(mp_obj_t obj);
 void validate_list_is_free_pins(qstr what, mcu_pin_obj_t **pins_out, mp_int_t max_pins, mp_obj_t seq, uint8_t *count_out);
+void validate_pins(qstr what, uint8_t *pin_nos, mp_int_t max_pins, mp_obj_t seq, uint8_t *count_out);
 
 void assert_pin_free(const mcu_pin_obj_t *pin);
 

shared-bindings/rgbmatrix/RGBMatrix.c

@@ -49,14 +49,6 @@ STATIC uint8_t validate_pin(mp_obj_t obj) {
     return common_hal_mcu_pin_number(result);
 }
 
-STATIC void validate_pins(qstr what, uint8_t *pin_nos, mp_int_t max_pins, mp_obj_t seq, uint8_t *count_out) {
-    mcu_pin_obj_t *pins[max_pins];
-    validate_list_is_free_pins(what, pins, max_pins, seq, count_out);
-    for (mp_int_t i = 0; i < *count_out; i++) {
-        pin_nos[i] = common_hal_mcu_pin_number(pins[i]);
-    }
-}
-
 STATIC void claim_and_never_reset_pin(mp_obj_t pin) {
     common_hal_mcu_pin_claim(pin);
     common_hal_never_reset_pin(pin);