Improve type validation errors messages, especially for pins

Author: dhalbert

locale/circuitpython.pot

@@ -105,6 +105,10 @@ msgstr ""
 msgid "%q failure: %d"
 msgstr ""
 
+#: py/argcheck.c
+msgid "%q in %q must be of type %q, not %q"
+msgstr ""
+
 #: ports/espressif/common-hal/espulp/ULP.c
 #: ports/raspberrypi/common-hal/rp2pio/StateMachine.c
 #: shared-bindings/digitalio/DigitalInOut.c
@@ -172,12 +176,13 @@ msgstr ""
 msgid "%q must be a bytearray or array of type 'h', 'H', 'b', or 'B'"
 msgstr ""
 
-#: py/argcheck.c py/obj.c py/objstrunicode.c
-msgid "%q must be of type %q"
+#: ports/raspberrypi/bindings/cyw43/__init__.c py/argcheck.c py/objexcept.c
+#: shared-bindings/canio/CAN.c shared-bindings/digitalio/Pull.c
+msgid "%q must be of type %q or %q, not %q"
 msgstr ""
 
-#: py/objexcept.c shared-bindings/digitalio/Pull.c
-msgid "%q must be of type %q or None"
+#: py/argcheck.c py/obj.c py/objstrunicode.c
+msgid "%q must be of type %q, not %q"
 msgstr ""
 
 #: ports/atmel-samd/common-hal/busio/UART.c
@@ -932,20 +937,8 @@ msgstr ""
 msgid "Error: Failure to bind"
 msgstr ""
 
-#: ports/espressif/bindings/espulp/ULP.c py/enum.c
-#: shared-bindings/_bleio/__init__.c shared-bindings/aesio/aes.c
-#: shared-bindings/alarm/__init__.c shared-bindings/busio/SPI.c
-#: shared-bindings/microcontroller/Pin.c
-#: shared-bindings/neopixel_write/__init__.c
-msgid "Expected a %q"
-msgstr ""
-
-#: ports/raspberrypi/bindings/cyw43/__init__.c
-msgid "Expected a %q or %q"
-msgstr ""
-
 #: shared-bindings/alarm/__init__.c
-msgid "Expected an %q"
+msgid "Expected a kind of %q"
 msgstr ""
 
 #: ports/espressif/common-hal/_bleio/Adapter.c
@@ -1118,11 +1111,6 @@ msgstr ""
 msgid "I2SOut not available"
 msgstr ""
 
-#: shared-bindings/aesio/aes.c
-#, c-format
-msgid "IV must be %d bytes long"
-msgstr ""
-
 #: ports/raspberrypi/bindings/rp2pio/StateMachine.c
 msgid "In-buffer elements must be <= 4 bytes long"
 msgstr ""
@@ -1583,10 +1571,6 @@ msgstr ""
 msgid "Not playing"
 msgstr ""
 
-#: shared-bindings/_bleio/__init__.c
-msgid "Not settable"
-msgstr ""
-
 #: ports/espressif/common-hal/paralleldisplay/ParallelBus.c
 #, c-format
 msgid "Number of data_pins must be 8 or 16, not %d"
@@ -1859,6 +1843,7 @@ msgstr ""
 msgid "Random number generation error"
 msgstr ""
 
+#: shared-bindings/_bleio/__init__.c
 #: shared-bindings/memorymonitor/AllocationSize.c
 #: shared-bindings/pulseio/PulseIn.c shared-module/displayio/Bitmap.c
 msgid "Read-only"
@@ -2045,7 +2030,10 @@ msgstr ""
 msgid "The above exception was the direct cause of the following exception:"
 msgstr ""
 
+#: ports/espressif/boards/m5stack_atom_echo/mpconfigboard.h
 #: ports/espressif/boards/m5stack_atom_lite/mpconfigboard.h
+#: ports/espressif/boards/m5stack_atom_matrix/mpconfigboard.h
+#: ports/espressif/boards/m5stack_atom_u/mpconfigboard.h
 msgid "The central button was pressed at start up.\n"
 msgstr ""
 
@@ -2969,14 +2957,6 @@ msgstr ""
 msgid "exceptions must derive from BaseException"
 msgstr ""
 
-#: shared-bindings/canio/CAN.c
-msgid "expected '%q' but got '%q'"
-msgstr ""
-
-#: shared-bindings/canio/CAN.c
-msgid "expected '%q' or '%q' but got '%q'"
-msgstr ""
-
 #: py/objstr.c
 msgid "expected ':' after format specifier"
 msgstr ""

ports/espressif/bindings/esp32_camera/Camera.c

@@ -121,14 +121,21 @@ STATIC mp_obj_t esp32_camera_camera_make_new(const mp_obj_type_t *type, size_t n
     validate_pins(MP_QSTR_data_pins, data_pins, MP_ARRAY_SIZE(data_pins), args[ARG_data_pins].u_obj, &data_pin_count);
     mp_arg_validate_length(data_pin_count, 8, MP_QSTR_data_pins);
 
-    const mcu_pin_obj_t *pixel_clock_pin = validate_obj_is_free_pin(args[ARG_pixel_clock_pin].u_obj);
-    const mcu_pin_obj_t *vsync_pin = validate_obj_is_free_pin(args[ARG_vsync_pin].u_obj);
-    const mcu_pin_obj_t *href_pin = validate_obj_is_free_pin(args[ARG_href_pin].u_obj);
+    const mcu_pin_obj_t *pixel_clock_pin =
+        validate_obj_is_free_pin(args[ARG_pixel_clock_pin].u_obj, MP_QSTR_pixel_clock_pin);
+    const mcu_pin_obj_t *vsync_pin =
+        validate_obj_is_free_pin(args[ARG_vsync_pin].u_obj, MP_QSTR_vsync_pin);
+    const mcu_pin_obj_t *href_pin =
+        validate_obj_is_free_pin(args[ARG_href_pin].u_obj, MP_QSTR_href_pin);
     busio_i2c_obj_t *i2c = MP_OBJ_TO_PTR(mp_arg_validate_type(args[ARG_i2c].u_obj, &busio_i2c_type, MP_QSTR_i2c));
-    const mcu_pin_obj_t *external_clock_pin = validate_obj_is_free_pin_or_none(args[ARG_external_clock_pin].u_obj);
-    const mcu_pin_obj_t *powerdown_pin = validate_obj_is_free_pin_or_none(args[ARG_powerdown_pin].u_obj);
-    const mcu_pin_obj_t *reset_pin = validate_obj_is_free_pin_or_none(args[ARG_reset_pin].u_obj);
-    const mp_int_t external_clock_frequency = mp_arg_validate_int_range(args[ARG_external_clock_frequency].u_int, 0, 40000000, MP_QSTR_clock_frequency);
+    const mcu_pin_obj_t *external_clock_pin =
+        validate_obj_is_free_pin_or_none(args[ARG_external_clock_pin].u_obj, MP_QSTR_external_clock_pin);
+    const mcu_pin_obj_t *powerdown_pin =
+        validate_obj_is_free_pin_or_none(args[ARG_powerdown_pin].u_obj, MP_QSTR_powerdown_pin);
+    const mcu_pin_obj_t *reset_pin =
+        validate_obj_is_free_pin_or_none(args[ARG_reset_pin].u_obj, MP_QSTR_reset_pin);
+    const mp_int_t external_clock_frequency =
+        mp_arg_validate_int_range(args[ARG_external_clock_frequency].u_int, 0, 40000000, MP_QSTR_external_clock_frequency);
 
     camera_grab_mode_t grab_mode = validate_grab_mode(args[ARG_grab_mode].u_obj, MP_QSTR_grab_mode);
     framesize_t frame_size = validate_frame_size(args[ARG_frame_size].u_obj, MP_QSTR_frame_size);

ports/espressif/bindings/esp32_camera/__init__.c

@@ -69,7 +69,7 @@ MAKE_PRINTER(esp32_camera, esp32_camera_grab_mode);
 MAKE_ENUM_TYPE(esp32_camera, GrabMode, esp32_camera_grab_mode);
 
 camera_grab_mode_t validate_grab_mode(mp_obj_t obj, qstr arg_name) {
-    return cp_enum_value(&esp32_camera_grab_mode_type, mp_arg_validate_type(obj, &esp32_camera_grab_mode_type, arg_name));
+    return cp_enum_value(&esp32_camera_grab_mode_type, obj, arg_name);
 }
 
 //| class PixelFormat:
@@ -100,7 +100,7 @@ MAKE_PRINTER(esp32_camera, esp32_camera_pixel_format);
 MAKE_ENUM_TYPE(esp32_camera, PixelFormat, esp32_camera_pixel_format);
 
 pixformat_t validate_pixel_format(mp_obj_t obj, qstr arg_name) {
-    return cp_enum_value(&esp32_camera_pixel_format_type, mp_arg_validate_type(obj, &esp32_camera_pixel_format_type, arg_name));
+    return cp_enum_value(&esp32_camera_pixel_format_type, obj, arg_name);
 }
 
 //| class FrameSize:
@@ -225,7 +225,7 @@ MAKE_PRINTER(esp32_camera, esp32_camera_frame_size);
 MAKE_ENUM_TYPE(esp32_camera, FrameSize, esp32_camera_frame_size);
 
 framesize_t validate_frame_size(mp_obj_t obj, qstr arg_name) {
-    return cp_enum_value(&esp32_camera_frame_size_type, mp_arg_validate_type(obj, &esp32_camera_frame_size_type, arg_name));
+    return cp_enum_value(&esp32_camera_frame_size_type, obj, arg_name);
 }
 
 //| class GainCeiling:
@@ -265,7 +265,7 @@ MAKE_PRINTER(esp32_camera, esp32_camera_gain_ceiling);
 MAKE_ENUM_TYPE(esp32_camera, GainCeiling, esp32_camera_gain_ceiling);
 
 gainceiling_t validate_gain_ceiling(mp_obj_t obj, qstr arg_name) {
-    return cp_enum_value(&esp32_camera_gain_ceiling_type, mp_arg_validate_type(obj, &esp32_camera_gain_ceiling_type, arg_name));
+    return cp_enum_value(&esp32_camera_gain_ceiling_type, obj, arg_name);
 }
 
 STATIC const mp_rom_map_elem_t esp32_camera_module_globals_table[] = {

ports/espressif/bindings/espulp/ULP.c

@@ -44,22 +44,17 @@ STATIC mp_obj_t espulp_ulp_make_new(const mp_obj_type_t *type, size_t n_args, si
     return MP_OBJ_FROM_PTR(self);
 }
 
-STATIC espulp_ulp_obj_t *get_ulp_obj(mp_obj_t self_in) {
-    if (!mp_obj_is_type(self_in, &espulp_ulp_type)) {
-        mp_raise_TypeError_varg(translate("Expected a %q"), MP_QSTR_ULP);
-    }
-    espulp_ulp_obj_t *self = MP_OBJ_TO_PTR(self_in);
+STATIC void check_for_deinit(espulp_ulp_obj_t *self) {
     if (common_hal_espulp_ulp_deinited(self)) {
         raise_deinited_error();
     }
-    return self;
 }
 
 //|     def deinit(self) -> None:
 //|         """Deinitialises the ULP and releases it for another program."""
 //|         ...
 STATIC mp_obj_t espulp_ulp_deinit(mp_obj_t self_in) {
-    espulp_ulp_obj_t *self = get_ulp_obj(self_in);
+    espulp_ulp_obj_t *self = MP_OBJ_TO_PTR(self_in);
     common_hal_espulp_ulp_deinit(self);
     return mp_const_none;
 }
@@ -89,14 +84,16 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(espulp_ulp___exit___obj, 4, 4, espulp
 //|         The program will continue to run even when the running Python is halted."""
 //|         ...
 STATIC mp_obj_t espulp_ulp_run(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
+    espulp_ulp_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]);
+    check_for_deinit(self);
+
     enum { ARG_program, ARG_pins };
     static const mp_arg_t allowed_args[] = {
         { MP_QSTR_program, MP_ARG_REQUIRED | MP_ARG_OBJ},
         { MP_QSTR_pins, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = mp_const_empty_tuple} },
     };
 
     mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
-    espulp_ulp_obj_t *self = get_ulp_obj(pos_args[0]);
     mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
 
     mp_buffer_info_t bufinfo;
@@ -111,7 +108,7 @@ STATIC mp_obj_t espulp_ulp_run(size_t n_args, const mp_obj_t *pos_args, mp_map_t
 
     for (mp_uint_t i = 0; i < num_pins; i++) {
         mp_obj_t pin_obj = mp_obj_subscr(pins_in, MP_OBJ_NEW_SMALL_INT(i), MP_OBJ_SENTINEL);
-        validate_obj_is_free_pin(pin_obj);
+        validate_obj_is_free_pin(pin_obj, MP_QSTR_pin);
         const mcu_pin_obj_t *pin = ((const mcu_pin_obj_t *)pin_obj);
         if (pin->number >= 32) {
             raise_ValueError_invalid_pin();
@@ -129,7 +126,10 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_KW(espulp_ulp_run_obj, 2, espulp_ulp_run);
 //|         ...
 //|
 STATIC mp_obj_t espulp_ulp_halt(mp_obj_t self_in) {
-    common_hal_espulp_ulp_halt(get_ulp_obj(self_in));
+    espulp_ulp_obj_t *self = MP_OBJ_TO_PTR(self_in);
+    check_for_deinit(self);
+
+    common_hal_espulp_ulp_halt(self);
     return mp_const_none;
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_1(espulp_ulp_halt_obj, espulp_ulp_halt);

ports/espressif/bindings/espulp/__init__.c

@@ -61,7 +61,7 @@
 //|
 
 STATIC mp_obj_t espulp_get_rtc_gpio_number(mp_obj_t pin_obj) {
-    const mcu_pin_obj_t *pin = validate_obj_is_pin(pin_obj);
+    const mcu_pin_obj_t *pin = validate_obj_is_pin(pin_obj, MP_QSTR_pin);
     mp_int_t number = common_hal_espulp_get_rtc_gpio_number(pin);
     if (number < 0) {
         return mp_const_none;

ports/espressif/common-hal/paralleldisplay/ParallelBus.c

@@ -118,7 +118,8 @@ void common_hal_paralleldisplay_parallelbus_construct(paralleldisplay_parallelbu
     const mcu_pin_obj_t *data_pins[8];
     for (int i = 0; i < 8; i++) {
         snprintf(buf, sizeof(buf), "GPIO%d", data0->number + i);
-        data_pins[i] = validate_obj_is_free_pin(mp_obj_dict_get(MP_OBJ_FROM_PTR(&mcu_pin_globals), mp_obj_new_str(buf, strlen(buf))));
+        data_pins[i] = validate_obj_is_free_pin(
+            mp_obj_dict_get(MP_OBJ_FROM_PTR(&mcu_pin_globals), mp_obj_new_str(buf, strlen(buf))), MP_QSTR_pin);
     }
     common_hal_paralleldisplay_parallelbus_construct_nonsequential(self, 8, data_pins, command, chip_select, write, read, reset, frequency);
 }

ports/raspberrypi/bindings/cyw43/__init__.c

@@ -116,23 +116,23 @@ STATIC mp_obj_t cyw43_get_power_management() {
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_0(cyw43_get_power_management_obj, cyw43_get_power_management);
 
-const mcu_pin_obj_t *validate_obj_is_pin_including_cyw43(mp_obj_t obj) {
+const mcu_pin_obj_t *validate_obj_is_pin_including_cyw43(mp_obj_t obj, qstr arg_name) {
     if (!mp_obj_is_type(obj, &mcu_pin_type) && !mp_obj_is_type(obj, &cyw43_pin_type)) {
-        mp_raise_TypeError_varg(translate("Expected a %q or %q"), mcu_pin_type.name, cyw43_pin_type.name);
+        mp_raise_TypeError_varg(translate("%q must be of type %q or %q, not %q"), arg_name, mcu_pin_type.name, cyw43_pin_type.name, mp_obj_get_type(obj)->name);
     }
     return MP_OBJ_TO_PTR(obj);
 }
 
-const mcu_pin_obj_t *validate_obj_is_free_pin_or_gpio29(mp_obj_t obj) {
-    const mcu_pin_obj_t *pin = validate_obj_is_pin(obj);
+const mcu_pin_obj_t *validate_obj_is_free_pin_or_gpio29(mp_obj_t obj, qstr arg_name) {
+    const mcu_pin_obj_t *pin = validate_obj_is_pin(obj, arg_name);
     if (obj != &pin_GPIO29) {
         assert_pin_free(pin);
     }
     return pin;
 }
 
-const mcu_pin_obj_t *validate_obj_is_free_pin_including_cyw43(mp_obj_t obj) {
-    const mcu_pin_obj_t *pin = validate_obj_is_pin_including_cyw43(obj);
+const mcu_pin_obj_t *validate_obj_is_free_pin_including_cyw43(mp_obj_t obj, qstr arg_name) {
+    const mcu_pin_obj_t *pin = validate_obj_is_pin_including_cyw43(obj, arg_name);
     assert_pin_free(pin);
     return pin;
 }

ports/raspberrypi/bindings/cyw43/__init__.h

@@ -31,9 +31,9 @@
 #include "common-hal/microcontroller/Pin.h"
 
 extern const mp_obj_type_t cyw43_pin_type;
-const mcu_pin_obj_t *validate_obj_is_free_pin_including_cyw43(mp_obj_t obj);
-const mcu_pin_obj_t *validate_obj_is_free_pin_or_gpio29(mp_obj_t obj);
-const mcu_pin_obj_t *validate_obj_is_pin_including_cyw43(mp_obj_t obj);
+const mcu_pin_obj_t *validate_obj_is_free_pin_including_cyw43(mp_obj_t obj, qstr arg_name);
+const mcu_pin_obj_t *validate_obj_is_free_pin_or_gpio29(mp_obj_t obj, qstr arg_name);
+const mcu_pin_obj_t *validate_obj_is_pin_including_cyw43(mp_obj_t obj, qstr arg_name);
 
 #define CONSTANT_CYW43_PM_VALUE(pm_mode, pm2_sleep_ret_ms, li_beacon_period, li_dtim_period, li_assoc) \
     (li_assoc << 20 | /* listen interval sent to ap */ \

ports/raspberrypi/bindings/rp2pio/StateMachine.c

@@ -222,19 +222,25 @@ STATIC mp_obj_t rp2pio_statemachine_make_new(const mp_obj_type_t *type, size_t n
     mp_get_buffer(args[ARG_init].u_obj, &init_bufinfo, MP_BUFFER_READ);
 
     // We don't validate pin in use here because we may be ok sharing them within a PIO.
-    const mcu_pin_obj_t *first_out_pin = validate_obj_is_pin_or_none(args[ARG_first_out_pin].u_obj);
+    const mcu_pin_obj_t *first_out_pin =
+        validate_obj_is_pin_or_none(args[ARG_first_out_pin].u_obj, MP_QSTR_first_out_pin);
     mp_int_t out_pin_count = mp_arg_validate_int_min(args[ARG_out_pin_count].u_int, 1, MP_QSTR_out_pin_count);
 
-    const mcu_pin_obj_t *first_in_pin = validate_obj_is_pin_or_none(args[ARG_first_in_pin].u_obj);
+    const mcu_pin_obj_t *first_in_pin =
+        validate_obj_is_pin_or_none(args[ARG_first_in_pin].u_obj, MP_QSTR_first_in_pin);
     mp_int_t in_pin_count = mp_arg_validate_int_min(args[ARG_in_pin_count].u_int, 1, MP_QSTR_in_pin_count);
 
-    const mcu_pin_obj_t *first_set_pin = validate_obj_is_pin_or_none(args[ARG_first_set_pin].u_obj);
+    const mcu_pin_obj_t *first_set_pin =
+        validate_obj_is_pin_or_none(args[ARG_first_set_pin].u_obj, MP_QSTR_first_set_pin);
     mp_int_t set_pin_count = mp_arg_validate_int_range(args[ARG_set_pin_count].u_int, 1, 5, MP_QSTR_set_pin_count);
 
-    const mcu_pin_obj_t *first_sideset_pin = validate_obj_is_pin_or_none(args[ARG_first_sideset_pin].u_obj);
-    mp_int_t sideset_pin_count = mp_arg_validate_int_range(args[ARG_sideset_pin_count].u_int, 1, 5, MP_QSTR_set_pin_count);
+    const mcu_pin_obj_t *first_sideset_pin =
+        validate_obj_is_pin_or_none(args[ARG_first_sideset_pin].u_obj, MP_QSTR_first_sideset_pin);
+    mp_int_t sideset_pin_count =
+        mp_arg_validate_int_range(args[ARG_sideset_pin_count].u_int, 1, 5, MP_QSTR_set_pin_count);
 
-    const mcu_pin_obj_t *jmp_pin = validate_obj_is_pin_or_none(args[ARG_jmp_pin].u_obj);
+    const mcu_pin_obj_t *jmp_pin =
+        validate_obj_is_pin_or_none(args[ARG_jmp_pin].u_obj, MP_QSTR_jmp_pin);
     digitalio_pull_t jmp_pin_pull = validate_pull(args[ARG_jmp_pin_pull].u_rom_obj, MP_QSTR_jmp_pull);
 
     mp_int_t pull_threshold =

ports/raspberrypi/bindings/rp2pio/__init__.c

@@ -43,11 +43,17 @@
 //|     """Return True if the pins have sequential GPIO numbers, False otherwise"""
 //|     ...
 //|
-STATIC mp_obj_t rp2pio_pins_are_sequential(const mp_obj_t pins) {
+STATIC mp_obj_t rp2pio_pins_are_sequential(mp_obj_t pins_obj) {
     size_t len;
     mp_obj_t *items;
-    mp_obj_get_array(pins, &len, &items);
-    return mp_obj_new_bool(common_hal_rp2pio_pins_are_sequential(len, items));
+    mp_obj_get_array(pins_obj, &len, &items);
+
+    const mcu_pin_obj_t *pins[len];
+    for (size_t i = 0; i < len; i++) {
+        pins[i] = validate_obj_is_pin(items[i], MP_QSTR_pins);
+    }
+
+    return mp_obj_new_bool(common_hal_rp2pio_pins_are_sequential(len, pins));
 }
 
 STATIC MP_DEFINE_CONST_FUN_OBJ_1(rp2pio_pins_are_sequential_obj, rp2pio_pins_are_sequential);