Skip to content

Added option to allow Displays with Single Byte Boundaries #1708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Mar 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ports/atmel-samd/boards/hallowing_m0_express/board.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ void board_init(void) {
0x37, // set vertical scroll command
display_init_sequence,
sizeof(display_init_sequence),
&pin_PA00);
&pin_PA00,
false); // single_byte_bounds
common_hal_displayio_display_set_auto_brightness(display, true);
}

Expand Down
3 changes: 2 additions & 1 deletion ports/atmel-samd/boards/pybadge/board.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ void board_init(void) {
0x37, // set vertical scroll command
display_init_sequence,
sizeof(display_init_sequence),
&pin_PA00);
&pin_PA00,
false); // single_byte_bounds
common_hal_displayio_display_set_auto_brightness(display, true);
}

Expand Down
4 changes: 3 additions & 1 deletion ports/atmel-samd/boards/pyportal/board.c
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ void board_init(void) {
0x37, // Set vertical scroll command
display_init_sequence,
sizeof(display_init_sequence),
&pin_PB31);
&pin_PB31,
false); // single_byte_bounds

common_hal_displayio_display_set_auto_brightness(display, true);
}

Expand Down
9 changes: 6 additions & 3 deletions shared-bindings/displayio/Display.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
//| Most people should not use this class directly. Use a specific display driver instead that will
//| contain the initialization sequence at minimum.
//|
//| .. class:: Display(display_bus, init_sequence, *, width, height, colstart=0, rowstart=0, rotation=0, color_depth=16, set_column_command=0x2a, set_row_command=0x2b, write_ram_command=0x2c, set_vertical_scroll=0, backlight_pin=None)
//| .. class:: Display(display_bus, init_sequence, *, width, height, colstart=0, rowstart=0, rotation=0, color_depth=16, set_column_command=0x2a, set_row_command=0x2b, write_ram_command=0x2c, set_vertical_scroll=0, backlight_pin=None, single_byte_bounds=False)
//|
//| Create a Display object on the given display bus (`displayio.FourWire` or `displayio.ParallelBus`).
//|
Expand Down Expand Up @@ -91,9 +91,10 @@
//| :param int write_ram_command: Command used to write pixels values into the update region
//| :param int set_vertical_scroll: Command used to set the first row to show
//| :param microcontroller.Pin backlight_pin: Pin connected to the display's backlight
//| :param bool single_byte_bounds: Display column and row commands use single bytes
//|
STATIC mp_obj_t displayio_display_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_display_bus, ARG_init_sequence, ARG_width, ARG_height, ARG_colstart, ARG_rowstart, ARG_rotation, ARG_color_depth, ARG_set_column_command, ARG_set_row_command, ARG_write_ram_command, ARG_set_vertical_scroll, ARG_backlight_pin };
enum { ARG_display_bus, ARG_init_sequence, ARG_width, ARG_height, ARG_colstart, ARG_rowstart, ARG_rotation, ARG_color_depth, ARG_set_column_command, ARG_set_row_command, ARG_write_ram_command, ARG_set_vertical_scroll, ARG_backlight_pin, ARG_single_byte_bounds };
static const mp_arg_t allowed_args[] = {
{ MP_QSTR_display_bus, MP_ARG_REQUIRED | MP_ARG_OBJ },
{ MP_QSTR_init_sequence, MP_ARG_REQUIRED | MP_ARG_OBJ },
Expand All @@ -108,6 +109,7 @@ STATIC mp_obj_t displayio_display_make_new(const mp_obj_type_t *type, size_t n_a
{ MP_QSTR_write_ram_command, MP_ARG_INT | MP_ARG_KW_ONLY, {.u_int = 0x2c} },
{ MP_QSTR_set_vertical_scroll, MP_ARG_INT | MP_ARG_KW_ONLY, {.u_int = 0x0} },
{ MP_QSTR_backlight_pin, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_obj = mp_const_none} },
{ MP_QSTR_single_byte_bounds, MP_ARG_BOOL | MP_ARG_KW_ONLY, {.u_bool = false} },
};
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);
Expand Down Expand Up @@ -146,7 +148,8 @@ STATIC mp_obj_t displayio_display_make_new(const mp_obj_type_t *type, size_t n_a
args[ARG_color_depth].u_int, args[ARG_set_column_command].u_int, args[ARG_set_row_command].u_int,
args[ARG_write_ram_command].u_int,
args[ARG_set_vertical_scroll].u_int,
bufinfo.buf, bufinfo.len, MP_OBJ_TO_PTR(backlight_pin));
bufinfo.buf, bufinfo.len, MP_OBJ_TO_PTR(backlight_pin),
args[ARG_single_byte_bounds].u_bool);

return self;
}
Expand Down
2 changes: 1 addition & 1 deletion shared-bindings/displayio/Display.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ void common_hal_displayio_display_construct(displayio_display_obj_t* self,
mp_obj_t bus, uint16_t width, uint16_t height,
int16_t colstart, int16_t rowstart, uint16_t rotation, uint16_t color_depth,
uint8_t set_column_command, uint8_t set_row_command, uint8_t write_ram_command, uint8_t set_vertical_scroll,
uint8_t* init_sequence, uint16_t init_sequence_len, const mcu_pin_obj_t* backlight_pin);
uint8_t* init_sequence, uint16_t init_sequence_len, const mcu_pin_obj_t* backlight_pin, bool single_byte_bounds);

int32_t common_hal_displayio_display_wait_for_frame(displayio_display_obj_t* self);

Expand Down
26 changes: 18 additions & 8 deletions shared-module/displayio/Display.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ void common_hal_displayio_display_construct(displayio_display_obj_t* self,
mp_obj_t bus, uint16_t width, uint16_t height, int16_t colstart, int16_t rowstart, uint16_t rotation,
uint16_t color_depth, uint8_t set_column_command, uint8_t set_row_command,
uint8_t write_ram_command, uint8_t set_vertical_scroll, uint8_t* init_sequence, uint16_t init_sequence_len,
const mcu_pin_obj_t* backlight_pin) {
const mcu_pin_obj_t* backlight_pin, bool single_byte_bounds) {
self->color_depth = color_depth;
self->set_column_command = set_column_command;
self->set_row_command = set_row_command;
Expand All @@ -54,6 +54,7 @@ void common_hal_displayio_display_construct(displayio_display_obj_t* self,
self->colstart = colstart;
self->rowstart = rowstart;
self->auto_brightness = false;
self->single_byte_bounds = single_byte_bounds;

if (MP_OBJ_IS_TYPE(bus, &displayio_parallelbus_type)) {
self->begin_transaction = common_hal_displayio_parallelbus_begin_transaction;
Expand Down Expand Up @@ -211,16 +212,25 @@ void displayio_display_end_transaction(displayio_display_obj_t* self) {
}

void displayio_display_set_region_to_update(displayio_display_obj_t* self, uint16_t x0, uint16_t y0, uint16_t x1, uint16_t y1) {
// TODO(tannewt): Handle displays with single byte bounds.
uint16_t data[2];
self->send(self->bus, true, &self->set_column_command, 1);
data[0] = __builtin_bswap16(x0 + self->colstart);
data[1] = __builtin_bswap16(x1 - 1 + self->colstart);
self->send(self->bus, false, (uint8_t*) data, 4);
if (self->single_byte_bounds) {
data[0] = __builtin_bswap16(((x0 + self->colstart) << 8) | ((x1 - 1 + self->colstart) & 0xFF));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using bswap and shifts here, put the data declaration inside the if and change it to uint8_t. That will isolate the individual values and make it clearer to read.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point on readability. I was just hoping it would make the program a little smaller when I did it this way. Thanks for merging it in. I can make the readability improvement for the next release.

self->send(self->bus, false, (uint8_t*) data, 2);
} else {
data[0] = __builtin_bswap16(x0 + self->colstart);
data[1] = __builtin_bswap16(x1 - 1 + self->colstart);
self->send(self->bus, false, (uint8_t*) data, 4);
}
self->send(self->bus, true, &self->set_row_command, 1);
data[0] = __builtin_bswap16(y0 + self->rowstart);
data[1] = __builtin_bswap16(y1 - 1 + self->rowstart);
self->send(self->bus, false, (uint8_t*) data, 4);
if (self->single_byte_bounds) {
data[0] = __builtin_bswap16(((y0 + self->rowstart) << 8) | ((y1 - 1 + self->rowstart) & 0xFF));
self->send(self->bus, false, (uint8_t*) data, 2);
} else {
data[0] = __builtin_bswap16(y0 + self->rowstart);
data[1] = __builtin_bswap16(y1 - 1 + self->rowstart);
self->send(self->bus, false, (uint8_t*) data, 4);
}
self->send(self->bus, true, &self->write_ram_command, 1);
}

Expand Down
1 change: 1 addition & 0 deletions shared-module/displayio/Display.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ typedef struct {
uint64_t last_refresh;
int16_t colstart;
int16_t rowstart;
bool single_byte_bounds;
display_bus_begin_transaction begin_transaction;
display_bus_send send;
display_bus_end_transaction end_transaction;
Expand Down