Protomatter: Make all arguments kw-only, add rgb count and optional height checking

They're not readily distinguishable by type.

I also added the requested height optional parameter; this is checked
against the computed one.  It's not feasible to use this parameter to
artificailly reduce the number of used rows, because changes in the
underlying C protomatter library would be required.

Finally, I added a better error message when the number of RGB pins was
not what was expected.

1 files changed, 24 insertions, 10 deletions

diff --git a/shared-bindings/_protomatter/Protomatter.c b/shared-bindings/_protomatter/Protomatter.c
index cfc0ba6ed..bb4339079 100644
--- a/shared-bindings/_protomatter/Protomatter.c
+++ b/shared-bindings/_protomatter/Protomatter.c
@@ -177,17 +177,18 @@ STATIC void preflight_pins_or_throw(uint8_t clock_pin, uint8_t *rgb_pins, uint8_
 
 STATIC mp_obj_t protomatter_protomatter_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_width, ARG_bit_depth, ARG_rgb_list, ARG_addr_list,
-        ARG_clock_pin, ARG_latch_pin, ARG_output_enable_pin, ARG_doublebuffer, ARG_framebuffer };
+        ARG_clock_pin, ARG_latch_pin, ARG_output_enable_pin, ARG_doublebuffer, ARG_framebuffer, ARG_height };
     static const mp_arg_t allowed_args[] = {
-        { MP_QSTR_width, MP_ARG_INT | MP_ARG_REQUIRED },
-        { MP_QSTR_bit_depth, MP_ARG_INT | MP_ARG_REQUIRED },
-        { MP_QSTR_rgb_pins, MP_ARG_OBJ | MP_ARG_REQUIRED },
-        { MP_QSTR_addr_pins, MP_ARG_OBJ | MP_ARG_REQUIRED },
-        { MP_QSTR_clock_pin, MP_ARG_OBJ | MP_ARG_REQUIRED },
-        { MP_QSTR_latch_pin, MP_ARG_OBJ | MP_ARG_REQUIRED },
-        { MP_QSTR_output_enable_pin, MP_ARG_OBJ | MP_ARG_REQUIRED },
-        { MP_QSTR_doublebuffer, MP_ARG_BOOL, { .u_bool = true } },
-        { MP_QSTR_framebuffer, MP_ARG_OBJ, { .u_obj = mp_const_none } },
+        { MP_QSTR_width, MP_ARG_INT | MP_ARG_REQUIRED | MP_ARG_KW_ONLY },
+        { MP_QSTR_bit_depth, MP_ARG_INT | MP_ARG_REQUIRED | MP_ARG_KW_ONLY },
+        { MP_QSTR_rgb_pins, MP_ARG_OBJ | MP_ARG_REQUIRED | MP_ARG_KW_ONLY },
+        { MP_QSTR_addr_pins, MP_ARG_OBJ | MP_ARG_REQUIRED | MP_ARG_KW_ONLY },
+        { MP_QSTR_clock_pin, MP_ARG_OBJ | MP_ARG_REQUIRED | MP_ARG_KW_ONLY },
+        { MP_QSTR_latch_pin, MP_ARG_OBJ | MP_ARG_REQUIRED | MP_ARG_KW_ONLY },
+        { MP_QSTR_output_enable_pin, MP_ARG_OBJ | MP_ARG_REQUIRED | MP_ARG_KW_ONLY },
+        { MP_QSTR_doublebuffer, MP_ARG_BOOL | MP_ARG_KW_ONLY, { .u_bool = true } },
+        { MP_QSTR_framebuffer, MP_ARG_OBJ | MP_ARG_KW_ONLY, { .u_obj = mp_const_none } },
+        { MP_QSTR_height, MP_ARG_INT | MP_ARG_KW_ONLY, { .u_int = 0 } },
     };
     mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
     mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
@@ -208,6 +209,19 @@ STATIC mp_obj_t protomatter_protomatter_make_new(const mp_obj_type_t *type, size
     validate_pins(MP_QSTR_rgb_pins, rgb_pins, MP_ARRAY_SIZE(self->rgb_pins), args[ARG_rgb_list].u_obj, &rgb_count);
     validate_pins(MP_QSTR_addr_pins, addr_pins, MP_ARRAY_SIZE(self->addr_pins), args[ARG_addr_list].u_obj, &addr_count);
 
+    if (rgb_count % 6) {
+        mp_raise_ValueError_varg(translate("Must use a multiple of 6 rgb pins, not %d"), rgb_count);
+    }
+
+    // TODO(@jepler) Use fewer than all rows of pixels if height < computed_height
+    if (args[ARG_height].u_int != 0) {
+        int computed_height = (rgb_count / 3) << (addr_count);
+        if (computed_height != args[ARG_height].u_int) {
+            mp_raise_ValueError_varg(
+                translate("%d address pins and %d rgb pins indicate a height of %d, not %d"), addr_count, rgb_count, computed_height, args[ARG_height].u_int);
+        }
+    }
+
     preflight_pins_or_throw(clock_pin, rgb_pins, rgb_count, true);
 
     mp_obj_t framebuffer = args[ARG_framebuffer].u_obj;