adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
Other
4.11k stars 1.22k forks source link

Remove `set_vertical_scroll` from displayio #5189

Closed lesamouraipourpre closed 3 years ago

lesamouraipourpre commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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