From 28bd29eb42d61d0a7116b6c4295e166fcfbe7145 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Wed, 16 Dec 2020 13:48:07 -0600 Subject: displayio: ColorConverter: fix logic errors about transparent pixels The transparent_color field was never initialized. I _think_ this means its value was always set to 0, or the blackest of blacks. Instead, initialize it to the sentinel value, newly given the name NO_TRANSPARENT_COLOR. This exposed a second problem: The test for whether there was an existing transparent color was wrong (backwards). I am guessing that this was not found due to the first bug; since the converter had a transparent color, the correct test would have led to always getting the error "Only one color can be transparent at a time". Closes #3723 --- shared-module/displayio/ColorConverter.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'shared-module') diff --git a/shared-module/displayio/ColorConverter.c b/shared-module/displayio/ColorConverter.c index 80558d037..40dcd9d16 100644 --- a/shared-module/displayio/ColorConverter.c +++ b/shared-module/displayio/ColorConverter.c @@ -29,6 +29,8 @@ #include "py/misc.h" #include "py/runtime.h" +#define NO_TRANSPARENT_COLOR (0x1000000) + uint32_t displayio_colorconverter_dither_noise_1 (uint32_t n) { n = (n >> 13) ^ n; @@ -42,6 +44,7 @@ uint32_t displayio_colorconverter_dither_noise_2(uint32_t x, uint32_t y) { void common_hal_displayio_colorconverter_construct(displayio_colorconverter_t* self, bool dither) { self->dither = dither; + self->transparent_color = NO_TRANSPARENT_COLOR; } uint16_t displayio_colorconverter_compute_rgb565(uint32_t color_rgb888) { @@ -130,7 +133,7 @@ bool common_hal_displayio_colorconverter_get_dither(displayio_colorconverter_t* } void common_hal_displayio_colorconverter_make_transparent(displayio_colorconverter_t* self, uint32_t transparent_color) { - if (self->transparent_color >= 0x1000000) { + if (self->transparent_color != NO_TRANSPARENT_COLOR) { mp_raise_RuntimeError(translate("Only one color can be transparent at a time")); } self->transparent_color = transparent_color; @@ -138,8 +141,8 @@ void common_hal_displayio_colorconverter_make_transparent(displayio_colorconvert void common_hal_displayio_colorconverter_make_opaque(displayio_colorconverter_t* self, uint32_t transparent_color) { (void) transparent_color; - // 0x1000000 will never equal a valid color - self->transparent_color = 0x1000000; + // NO_TRANSPARENT_COLOR will never equal a valid color + self->transparent_color = NO_TRANSPARENT_COLOR; } void displayio_colorconverter_convert(displayio_colorconverter_t *self, const _displayio_colorspace_t* colorspace, const displayio_input_pixel_t *input_pixel, displayio_output_pixel_t *output_color) { -- cgit v1.2.3 From f224ed18488b1e92b6d13b09b696769a2ac380ce Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Thu, 17 Dec 2020 10:54:37 -0600 Subject: OnDiskBitmap: Correct handling of "0 color palette" images Microsoft documentation says: > If biCompression equals BI_RGB and the bitmap uses 8 bpp or less, the bitmap has a color table immediatelly following the BITMAPINFOHEADER structure. The color table consists of an array of RGBQUAD values. The size of the array is given by the biClrUsed member. If biClrUsed is zero, the array contains the maximum number of colors for the given bitdepth; that is, 2^biBitCount colors. Formerly, we treated 0 colors as "no image palette" during construction, but then during common_hal_displayio_ondiskbitmap_get_pixel indexed into the palette anyway. This could have unpredictable results. On a pygamer, it gave an image that was blue and black. On magtag, it gave a crash. --- shared-module/displayio/OnDiskBitmap.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'shared-module') diff --git a/shared-module/displayio/OnDiskBitmap.c b/shared-module/displayio/OnDiskBitmap.c index 993fc03de..c1a0ddeb7 100644 --- a/shared-module/displayio/OnDiskBitmap.c +++ b/shared-module/displayio/OnDiskBitmap.c @@ -57,7 +57,7 @@ void common_hal_displayio_ondiskbitmap_construct(displayio_ondiskbitmap_t *self, uint32_t compression = read_word(bmp_header, 15); uint32_t number_of_colors = read_word(bmp_header, 23); - bool indexed = ((bits_per_pixel <= 8) && (number_of_colors != 0)); + bool indexed = bits_per_pixel <= 8; self->bitfield_compressed = (compression == 3); self->bits_per_pixel = bits_per_pixel; self->width = read_word(bmp_header, 9); @@ -75,6 +75,9 @@ void common_hal_displayio_ondiskbitmap_construct(displayio_ondiskbitmap_t *self, self->b_bitmask = 0x1f; } } else if (indexed && self->bits_per_pixel != 1) { + if (number_of_colors == 0) { + number_of_colors = 1 << bits_per_pixel; + } uint16_t palette_size = number_of_colors * sizeof(uint32_t); uint16_t palette_offset = 0xe + header_size; -- cgit v1.2.3 From 6c4df5a8b45c6b20e00b5499c5db1d4f5da444a2 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Fri, 18 Dec 2020 11:13:28 -0600 Subject: adafruit_bus_device: Don't transmit STOP condition in write_then_readinto Closes #3795 --- shared-bindings/adafruit_bus_device/I2CDevice.c | 8 ++++---- shared-bindings/adafruit_bus_device/I2CDevice.h | 2 +- shared-module/adafruit_bus_device/I2CDevice.c | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) (limited to 'shared-module') diff --git a/shared-bindings/adafruit_bus_device/I2CDevice.c b/shared-bindings/adafruit_bus_device/I2CDevice.c index 4c9086162..a4c04e198 100644 --- a/shared-bindings/adafruit_bus_device/I2CDevice.c +++ b/shared-bindings/adafruit_bus_device/I2CDevice.c @@ -163,7 +163,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_KW(adafruit_bus_device_i2cdevice_readinto_obj, 2, //| """ //| ... //| -STATIC void write(adafruit_bus_device_i2cdevice_obj_t *self, mp_obj_t buffer, int32_t start, mp_int_t end) { +STATIC void write(adafruit_bus_device_i2cdevice_obj_t *self, mp_obj_t buffer, int32_t start, mp_int_t end, bool transmit_stop_bit) { mp_buffer_info_t bufinfo; mp_get_buffer_raise(buffer, &bufinfo, MP_BUFFER_READ); @@ -173,7 +173,7 @@ STATIC void write(adafruit_bus_device_i2cdevice_obj_t *self, mp_obj_t buffer, in mp_raise_ValueError(translate("Buffer must be at least length 1")); } - uint8_t status = common_hal_adafruit_bus_device_i2cdevice_write(MP_OBJ_TO_PTR(self), ((uint8_t*)bufinfo.buf) + start, length); + uint8_t status = common_hal_adafruit_bus_device_i2cdevice_write(MP_OBJ_TO_PTR(self), ((uint8_t*)bufinfo.buf) + start, length, transmit_stop_bit); if (status != 0) { mp_raise_OSError(status); } @@ -191,7 +191,7 @@ STATIC mp_obj_t adafruit_bus_device_i2cdevice_write(size_t n_args, const mp_obj_ mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)]; mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args); - write(self, args[ARG_buffer].u_obj, args[ARG_start].u_int, args[ARG_end].u_int); + write(self, args[ARG_buffer].u_obj, args[ARG_start].u_int, args[ARG_end].u_int, true); return mp_const_none; } MP_DEFINE_CONST_FUN_OBJ_KW(adafruit_bus_device_i2cdevice_write_obj, 2, adafruit_bus_device_i2cdevice_write); @@ -233,7 +233,7 @@ STATIC mp_obj_t adafruit_bus_device_i2cdevice_write_then_readinto(size_t n_args, mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)]; mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args); - write(self, args[ARG_out_buffer].u_obj, args[ARG_out_start].u_int, args[ARG_out_end].u_int); + write(self, args[ARG_out_buffer].u_obj, args[ARG_out_start].u_int, args[ARG_out_end].u_int, false); readinto(self, args[ARG_in_buffer].u_obj, args[ARG_in_start].u_int, args[ARG_in_end].u_int); diff --git a/shared-bindings/adafruit_bus_device/I2CDevice.h b/shared-bindings/adafruit_bus_device/I2CDevice.h index 7b2182ff0..cf7b1321a 100644 --- a/shared-bindings/adafruit_bus_device/I2CDevice.h +++ b/shared-bindings/adafruit_bus_device/I2CDevice.h @@ -45,7 +45,7 @@ extern const mp_obj_type_t adafruit_bus_device_i2cdevice_type; // Initializes the hardware peripheral. extern void common_hal_adafruit_bus_device_i2cdevice_construct(adafruit_bus_device_i2cdevice_obj_t *self, busio_i2c_obj_t *i2c, uint8_t device_address); extern uint8_t common_hal_adafruit_bus_device_i2cdevice_readinto(adafruit_bus_device_i2cdevice_obj_t *self, mp_obj_t buffer, size_t length); -extern uint8_t common_hal_adafruit_bus_device_i2cdevice_write(adafruit_bus_device_i2cdevice_obj_t *self, mp_obj_t buffer, size_t length); +extern uint8_t common_hal_adafruit_bus_device_i2cdevice_write(adafruit_bus_device_i2cdevice_obj_t *self, mp_obj_t buffer, size_t length, bool transmit_stop_bit); extern void common_hal_adafruit_bus_device_i2cdevice_lock(adafruit_bus_device_i2cdevice_obj_t *self); extern void common_hal_adafruit_bus_device_i2cdevice_unlock(adafruit_bus_device_i2cdevice_obj_t *self); extern void common_hal_adafruit_bus_device_i2cdevice_probe_for_device(adafruit_bus_device_i2cdevice_obj_t *self); diff --git a/shared-module/adafruit_bus_device/I2CDevice.c b/shared-module/adafruit_bus_device/I2CDevice.c index ee4b4fa55..83abe80d6 100644 --- a/shared-module/adafruit_bus_device/I2CDevice.c +++ b/shared-module/adafruit_bus_device/I2CDevice.c @@ -57,8 +57,8 @@ uint8_t common_hal_adafruit_bus_device_i2cdevice_readinto(adafruit_bus_device_i2 return common_hal_busio_i2c_read(self->i2c, self->device_address, buffer, length); } -uint8_t common_hal_adafruit_bus_device_i2cdevice_write(adafruit_bus_device_i2cdevice_obj_t *self, mp_obj_t buffer, size_t length) { - return common_hal_busio_i2c_write(self->i2c, self->device_address, buffer, length, true); +uint8_t common_hal_adafruit_bus_device_i2cdevice_write(adafruit_bus_device_i2cdevice_obj_t *self, mp_obj_t buffer, size_t length, bool transmit_stop_bit) { + return common_hal_busio_i2c_write(self->i2c, self->device_address, buffer, length, transmit_stop_bit); } void common_hal_adafruit_bus_device_i2cdevice_probe_for_device(adafruit_bus_device_i2cdevice_obj_t *self) { -- cgit v1.2.3 From dc332baa87bf1fa3d777ffe31c7dc29b24a6a142 Mon Sep 17 00:00:00 2001 From: microDev <70126934+microDev1@users.noreply.github.com> Date: Mon, 28 Dec 2020 20:04:00 +0530 Subject: update common_hal_reset_pin() --- ports/atmel-samd/common-hal/microcontroller/Pin.c | 3 +++ ports/esp32s2/common-hal/busio/SPI.c | 8 ++------ ports/esp32s2/common-hal/microcontroller/Pin.c | 3 +++ ports/litex/common-hal/microcontroller/Pin.c | 3 +++ ports/mimxrt10xx/common-hal/busio/SPI.c | 9 +++------ ports/mimxrt10xx/common-hal/busio/UART.c | 10 ++++------ ports/mimxrt10xx/common-hal/microcontroller/Pin.c | 3 +++ ports/nrf/common-hal/microcontroller/Pin.c | 3 +++ ports/stm/common-hal/microcontroller/Pin.c | 3 +++ shared-module/displayio/FourWire.c | 4 +--- 10 files changed, 28 insertions(+), 21 deletions(-) (limited to 'shared-module') diff --git a/ports/atmel-samd/common-hal/microcontroller/Pin.c b/ports/atmel-samd/common-hal/microcontroller/Pin.c index 7dd87a3b2..564d037e0 100644 --- a/ports/atmel-samd/common-hal/microcontroller/Pin.c +++ b/ports/atmel-samd/common-hal/microcontroller/Pin.c @@ -169,6 +169,9 @@ void common_hal_never_reset_pin(const mcu_pin_obj_t* pin) { } void common_hal_reset_pin(const mcu_pin_obj_t* pin) { + if (pin == NULL) { + return; + } reset_pin_number(pin->number); } diff --git a/ports/esp32s2/common-hal/busio/SPI.c b/ports/esp32s2/common-hal/busio/SPI.c index 562881585..62f47999c 100644 --- a/ports/esp32s2/common-hal/busio/SPI.c +++ b/ports/esp32s2/common-hal/busio/SPI.c @@ -248,12 +248,8 @@ void common_hal_busio_spi_deinit(busio_spi_obj_t *self) { spi_bus_free(self->host_id); common_hal_reset_pin(self->clock_pin); - if (self->MOSI_pin != NULL) { - common_hal_reset_pin(self->MOSI_pin); - } - if (self->MISO_pin != NULL) { - common_hal_reset_pin(self->MISO_pin); - } + common_hal_reset_pin(self->MOSI_pin); + common_hal_reset_pin(self->MISO_pin); self->clock_pin = NULL; self->MISO_pin = NULL; self->MOSI_pin = NULL; diff --git a/ports/esp32s2/common-hal/microcontroller/Pin.c b/ports/esp32s2/common-hal/microcontroller/Pin.c index 81dfa1308..26bd6c16b 100644 --- a/ports/esp32s2/common-hal/microcontroller/Pin.c +++ b/ports/esp32s2/common-hal/microcontroller/Pin.c @@ -89,6 +89,9 @@ void reset_pin_number(gpio_num_t pin_number) { } void common_hal_reset_pin(const mcu_pin_obj_t* pin) { + if (pin == NULL) { + return; + } reset_pin_number(pin->number); } diff --git a/ports/litex/common-hal/microcontroller/Pin.c b/ports/litex/common-hal/microcontroller/Pin.c index 38601e0d3..4d088a076 100644 --- a/ports/litex/common-hal/microcontroller/Pin.c +++ b/ports/litex/common-hal/microcontroller/Pin.c @@ -42,6 +42,9 @@ void reset_pin_number(uint8_t pin_port, uint8_t pin_number) { } void common_hal_reset_pin(const mcu_pin_obj_t* pin) { + if (pin == NULL) { + return; + } reset_pin_number(0, pin->number); } diff --git a/ports/mimxrt10xx/common-hal/busio/SPI.c b/ports/mimxrt10xx/common-hal/busio/SPI.c index 8fe1b799d..ce7cbea7e 100644 --- a/ports/mimxrt10xx/common-hal/busio/SPI.c +++ b/ports/mimxrt10xx/common-hal/busio/SPI.c @@ -220,12 +220,9 @@ void common_hal_busio_spi_deinit(busio_spi_obj_t *self) { never_reset_spi[self->clock->bank_idx - 1] = false; common_hal_reset_pin(self->clock->pin); - if (self->mosi != NULL) { - common_hal_reset_pin(self->mosi->pin); - } - if (self->miso != NULL) { - common_hal_reset_pin(self->miso->pin); - } + common_hal_reset_pin(self->mosi->pin); + common_hal_reset_pin(self->miso->pin); + self->clock = NULL; self->mosi = NULL; self->miso = NULL; diff --git a/ports/mimxrt10xx/common-hal/busio/UART.c b/ports/mimxrt10xx/common-hal/busio/UART.c index db5582d15..03a6e8f3e 100644 --- a/ports/mimxrt10xx/common-hal/busio/UART.c +++ b/ports/mimxrt10xx/common-hal/busio/UART.c @@ -295,12 +295,10 @@ void common_hal_busio_uart_deinit(busio_uart_obj_t *self) { LPUART_Deinit(self->uart); gc_free(self->ringbuf); - if (self->rx) { - common_hal_reset_pin(self->rx->pin); - } - if (self->tx) { - common_hal_reset_pin(self->tx->pin); - } + + common_hal_reset_pin(self->rx->pin); + common_hal_reset_pin(self->tx->pin); + self->rx = NULL; self->tx = NULL; diff --git a/ports/mimxrt10xx/common-hal/microcontroller/Pin.c b/ports/mimxrt10xx/common-hal/microcontroller/Pin.c index a005924e2..50970f8e2 100644 --- a/ports/mimxrt10xx/common-hal/microcontroller/Pin.c +++ b/ports/mimxrt10xx/common-hal/microcontroller/Pin.c @@ -67,6 +67,9 @@ void reset_all_pins(void) { // Since i.MX pins need extra register and reset information to reset properly, // resetting pins by number alone has been removed. void common_hal_reset_pin(const mcu_pin_obj_t* pin) { + if (pin == NULL) { + return; + } never_reset_pins[pin->mux_idx] = false; claimed_pins[pin->mux_idx] = false; *(uint32_t*)pin->mux_reg = pin->mux_reset; diff --git a/ports/nrf/common-hal/microcontroller/Pin.c b/ports/nrf/common-hal/microcontroller/Pin.c index bc7ff831a..23d6e23bf 100644 --- a/ports/nrf/common-hal/microcontroller/Pin.c +++ b/ports/nrf/common-hal/microcontroller/Pin.c @@ -134,6 +134,9 @@ void common_hal_never_reset_pin(const mcu_pin_obj_t* pin) { } void common_hal_reset_pin(const mcu_pin_obj_t* pin) { + if (pin == NULL) { + return; + } reset_pin_number(pin->number); } diff --git a/ports/stm/common-hal/microcontroller/Pin.c b/ports/stm/common-hal/microcontroller/Pin.c index 0e333c71c..37c202d86 100644 --- a/ports/stm/common-hal/microcontroller/Pin.c +++ b/ports/stm/common-hal/microcontroller/Pin.c @@ -125,6 +125,9 @@ void common_hal_never_reset_pin(const mcu_pin_obj_t* pin) { } void common_hal_reset_pin(const mcu_pin_obj_t* pin) { + if (pin == NULL) { + return; + } reset_pin_number(pin->port, pin->number); } diff --git a/shared-module/displayio/FourWire.c b/shared-module/displayio/FourWire.c index 06f8a84e3..e993f2480 100644 --- a/shared-module/displayio/FourWire.c +++ b/shared-module/displayio/FourWire.c @@ -76,9 +76,7 @@ void common_hal_displayio_fourwire_deinit(displayio_fourwire_obj_t* self) { common_hal_reset_pin(self->command.pin); common_hal_reset_pin(self->chip_select.pin); - if (self->reset.pin) { - common_hal_reset_pin(self->reset.pin); - } + common_hal_reset_pin(self->reset.pin); } bool common_hal_displayio_fourwire_reset(mp_obj_t obj) { -- cgit v1.2.3