Skip to content
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

Remove set_vertical_scroll from displayio #5189

Closed
lesamouraipourpre opened this issue Aug 20, 2021 · 3 comments · Fixed by #5205
Closed

Remove set_vertical_scroll from displayio #5189

lesamouraipourpre opened this issue Aug 20, 2021 · 3 comments · Fixed by #5205
Assignees
Milestone

Comments

@lesamouraipourpre
Copy link

As far as I can tell the set_vertical_scroll parameter in the Display constructor is not used.

grep -r set_vertical_scroll *
shared-bindings/displayio/Display.c://|     def __init__(self, display_bus: _DisplayBus, init_sequence: ReadableBuffer, *, width: int, height: int, colstart: int = 0, rowstart: int = 0, rotation: int = 0, color_depth: int = 16, grayscale: bool = False, pixels_in_byte_share_row: bool = True, bytes_per_cell: int = 1, reverse_pixels_in_byte: bool = False, set_column_command: int = 0x2a, set_row_command: int = 0x2b, write_ram_command: int = 0x2c, set_vertical_scroll: int = 0, backlight_pin: Optional[microcontroller.Pin] = None, brightness_command: Optional[int] = None, brightness: float = 1.0, auto_brightness: bool = False, single_byte_bounds: bool = False, data_as_commands: bool = False,  auto_refresh: bool = True, native_frames_per_second: int = 60, backlight_on_high: bool = True, SH1107_addressing: bool = False) -> None:
shared-bindings/displayio/Display.c://|         :param int set_vertical_scroll: Command used to set the first row to show
shared-bindings/displayio/Display.c:           ARG_set_vertical_scroll, ARG_backlight_pin, ARG_brightness_command,
shared-bindings/displayio/Display.c:        { MP_QSTR_set_vertical_scroll, MP_ARG_INT | MP_ARG_KW_ONLY, {.u_int = 0x0} },
shared-bindings/displayio/Display.c:        args[ARG_set_vertical_scroll].u_int,
shared-bindings/displayio/Display.h:    uint8_t set_column_command, uint8_t set_row_command, uint8_t write_ram_command, uint8_t set_vertical_scroll,
shared-module/displayio/Display.c:    uint8_t set_row_command, uint8_t write_ram_command, uint8_t set_vertical_scroll,

Should it be removed?

@tannewt tannewt added this to the 7.0.0 milestone Aug 20, 2021
@tannewt
Copy link
Member

tannewt commented Aug 20, 2021

I probably added it optimistically. (It would really help speed up scrolls in one direction.) In almost all cases I've looked at, the orientation is wrong. We can remove it in 7.0.0 if no libraries use it. We can always add it back if we start using it.

@lesamouraipourpre
Copy link
Author

Against the Adafruit bundle:

> grep -r set_vertical_scroll * | sort
libraries/drivers/displayio_sh1106/adafruit_displayio_sh1106.py:            set_vertical_scroll=0xD3,  # TBD -- not sure about this one!
libraries/drivers/displayio_sh1107/adafruit_displayio_sh1107.py:            set_vertical_scroll=0xD3,  # TBD -- not sure about this one!
libraries/drivers/displayio_ssd1305/adafruit_displayio_ssd1305.py:            set_vertical_scroll=0xD3,
libraries/drivers/displayio_ssd1306/adafruit_displayio_ssd1306.py:            set_vertical_scroll=0xD3,
libraries/drivers/ssd1322/adafruit_ssd1322.py:            set_vertical_scroll=0xD3,
libraries/drivers/ssd1325/adafruit_ssd1325.py:            set_vertical_scroll=0xD3,

If you are happy enough to remove it, I can do PRs for these 6 libraries.

@jepler
Copy link
Member

jepler commented Aug 23, 2021

Yes, let's mark it as deprecated and ignored in 7.0, then remove it from the core in 8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants