paralleldisplay: reset and read pins should be optional

The ``reset`` and ``read`` pins should be optional, but the espressif
code had several places where it assumed they are not, and a bug that
caused a crash on ``release_displays`` if they were made optional.

The bug was caused by the fields for storing pin numbers being set
to ``NO_PIN``, which has value of -1, while the fields have type
``uint8_t``.  That set the actual value to 255, and a subsequent
comparison to ``NO_PIN`` returned false.

Author: deshipu

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

@@ -72,13 +72,6 @@ void common_hal_paralleldisplay_parallelbus_construct_nonsequential(paralleldisp
         .buffer_size = 512,
     };
 
-    if (reset != NULL) {
-        common_hal_never_reset_pin(reset);
-        self->reset_pin_number = reset->number;
-    } else {
-        self->reset_pin_number = NO_PIN;
-    }
-
     for (uint8_t i = 0; i < n_pins; i++) {
         common_hal_never_reset_pin(data_pins[i]);
         config.pin_data_num[i] = common_hal_mcu_pin_number(data_pins[i]);
@@ -98,10 +91,14 @@ void common_hal_paralleldisplay_parallelbus_construct_nonsequential(paralleldisp
         gpio_set_level(read->number, true);
     }
 
+    self->reset_pin_number = NO_PIN;
+    if (reset != NULL) {
+        common_hal_never_reset_pin(reset);
+        self->reset_pin_number = reset->number;
+    }
+
     common_hal_never_reset_pin(chip_select);
     common_hal_never_reset_pin(command);
-    common_hal_never_reset_pin(read);
-    common_hal_never_reset_pin(reset);
     common_hal_never_reset_pin(write);
 
     self->config = config;
@@ -140,8 +137,8 @@ void common_hal_paralleldisplay_parallelbus_deinit(paralleldisplay_parallelbus_o
 
     reset_pin_number(self->config.pin_num_cs);
     reset_pin_number(self->config.pin_num_wr);
-    reset_pin_number(self->read_pin_number);
     reset_pin_number(self->config.pin_num_rs);
+    reset_pin_number(self->read_pin_number);
     reset_pin_number(self->reset_pin_number);
 
     port_i2s_reset_instance(0);

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

@@ -32,8 +32,8 @@
 
 typedef struct {
     mp_obj_base_t base;
-    uint8_t read_pin_number;
-    uint8_t reset_pin_number;
+    int8_t read_pin_number;
+    int8_t reset_pin_number;
     i2s_lcd_config_t config;
     i2s_lcd_handle_t handle;
 } paralleldisplay_parallelbus_obj_t;

shared-bindings/paralleldisplay/ParallelBus.c

@@ -42,7 +42,7 @@
 //|     protocol may be refered to as 8080-I Series Parallel Interface in datasheets. It doesn't handle
 //|     display initialization."""
 //|
-//|     def __init__(self, *, data0: microcontroller.Pin, command: microcontroller.Pin, chip_select: microcontroller.Pin, write: microcontroller.Pin, read: microcontroller.Pin, reset: microcontroller.Pin, frequency: int = 30_000_000) -> None:
+//|     def __init__(self, *, data0: microcontroller.Pin, command: microcontroller.Pin, chip_select: microcontroller.Pin, write: microcontroller.Pin, read: Optional[microcontroller.Pin], reset: Optional[microcontroller.Pin] = None, frequency: int = 30_000_000) -> None:
 //|         """Create a ParallelBus object associated with the given pins. The bus is inferred from data0
 //|         by implying the next 7 additional pins on a given GPIO port.
 //|
@@ -56,8 +56,8 @@
 //|         :param microcontroller.Pin command: Data or command pin
 //|         :param microcontroller.Pin chip_select: Chip select pin
 //|         :param microcontroller.Pin write: Write pin
-//|         :param microcontroller.Pin read: Read pin
-//|         :param microcontroller.Pin reset: Reset pin
+//|         :param microcontroller.Pin read: Read pin, optional
+//|         :param microcontroller.Pin reset: Reset pin, optional
 //|         :param int frequency: The communication frequency in Hz for the display on the bus"""
 //|         ...
 //|
@@ -69,8 +69,8 @@ STATIC mp_obj_t paralleldisplay_parallelbus_make_new(const mp_obj_type_t *type,
         { MP_QSTR_command, MP_ARG_OBJ | MP_ARG_KW_ONLY | MP_ARG_REQUIRED },
         { MP_QSTR_chip_select, MP_ARG_OBJ | MP_ARG_KW_ONLY | MP_ARG_REQUIRED },
         { MP_QSTR_write, MP_ARG_OBJ | MP_ARG_KW_ONLY | MP_ARG_REQUIRED },
-        { MP_QSTR_read, MP_ARG_OBJ | MP_ARG_KW_ONLY | MP_ARG_REQUIRED },
-        { MP_QSTR_reset, MP_ARG_OBJ | MP_ARG_KW_ONLY | MP_ARG_REQUIRED },
+        { MP_QSTR_read, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_obj = mp_const_none } },
+        { MP_QSTR_reset, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_obj = mp_const_none } },
         { MP_QSTR_frequency, MP_ARG_INT | MP_ARG_KW_ONLY, {.u_int = 30000000 } },
     };
     mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
@@ -79,8 +79,8 @@ STATIC mp_obj_t paralleldisplay_parallelbus_make_new(const mp_obj_type_t *type,
     const mcu_pin_obj_t *command = validate_obj_is_free_pin(args[ARG_command].u_obj);
     const mcu_pin_obj_t *chip_select = validate_obj_is_free_pin(args[ARG_chip_select].u_obj);
     const mcu_pin_obj_t *write = validate_obj_is_free_pin(args[ARG_write].u_obj);
-    const mcu_pin_obj_t *read = validate_obj_is_free_pin(args[ARG_read].u_obj);
-    const mcu_pin_obj_t *reset = validate_obj_is_free_pin(args[ARG_reset].u_obj);
+    const mcu_pin_obj_t *read = validate_obj_is_free_pin_or_none(args[ARG_read].u_obj);
+    const mcu_pin_obj_t *reset = validate_obj_is_free_pin_or_none(args[ARG_reset].u_obj);
 
     paralleldisplay_parallelbus_obj_t *self = &allocate_display_bus_or_raise()->parallel_bus;
     self->base.type = &paralleldisplay_parallelbus_type;