-
Notifications
You must be signed in to change notification settings - Fork 11
Added madctl parameter to ILI9341 driver for hardware rotation and RGB/BGR control, enabling faster rendering and more flexibility #43
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
base: main
Are you sure you want to change the base?
Conversation
1361b03 to
74a43af
Compare
|
Please review this commit |
|
reviewed. we will not merge, please use/add/adapt kwargs instead of hardcoding MADCTL writes |
74a43af to
a289fba
Compare
|
Hi @ladyada your review comment addressed please take a look. |
a289fba to
c49a3cb
Compare
|
hi, to be clear: we will not accept passing in MADCTL. instead, you can add kwargs for any/each setting that may be required (e.g. color order) |
150e56a to
7689f05
Compare
|
Hi @ladyada I’ve updated this PR as suggested: replaced raw madctl with a color_order kwarg (default "RGB", optional "BGR"), kept bgr for compatibility, and updated docs/tests. Ready for re-review. |
| """ | ||
| ILI9341 display driver | ||
| :param FourWire bus: bus that the display is connected to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The FourWire bus argument should not get removed from the docstrings I think
| *, | ||
| width=240, | ||
| height=320, | ||
| rotation=0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add docstring listings for the new args width, height, and rotation all arguments should be listed in the docstrings above.
| dependencies = {file = ["requirements.txt"]} | ||
| optional-dependencies = {optional = {file = ["optional_requirements.txt"]}} | ||
|
|
||
| [tool.ruff] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't generally want to make configuration changes to pyproject.toml in single repos like this.
We have ruff.toml file with relatively configuration inside of it that is used for all of the library repos.
If there are configuration changes that we should consider then we'll want to discuss doing so more broadly with a patch across all repos instead of having it changed one-off in PRs like this.
Please revert the changes in pyproject.toml for now.
Summary
This PR adds support for passing a
madctlparameter to theILI9341driver, allowing developers to directly control hardware rotation via the MADCTL register.It also preserves support for BGR/RGB selection and inversion.
Motivation
Currently, display rotation relies on displayio’s software-based handling, which can be inefficient.
By exposing MADCTL, developers can take advantage of the LCD controller’s built-in rotation logic, which improves performance (especially for OnDiskBitmaps).
Changes
madctlparameter to theILI9341constructor.bgrandinvertoptions.madctl.Tests
test_madctl.py) covering:Related Issues
Closes #26 ("Make it easy to set MADCTL for rotation").