Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added the ST7735 Display Driver #2
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
Added the ST7735 Display Driver #2
Changes from 22 commits
8c4514b
382dbbb
ca042ca
5a60f40
4ca2cea
e49fd35
06fc4a3
5cf39ce
b9df192
040d4ea
29d10c8
39b75b9
f4b6c8d
0fe8bda
8690f94
6921f75
521dabe
e9c3636
2c43ccb
7107fdb
2cd7054
1d3f15d
5662cb0
b984037
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I'm not a fan of this init argument because it leaks the binary blob init stuff. Display already has a rotation kwarg that can be used for rotation. (It's very slow for OnDiskBitmaps atm but we can improve it later.) If we also need a toggle for RGB or BGR then let's add it explicitly rather than generically.
By using Display for rotation we align our display memory accesses with the native refresh direction and make it easier to make updates tear-free in the future.
Advanced users can always use the display bus themselves to send extra commands too.
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.
I think the rotation doesn’t work for some displays like the 3.5 FeatherWing. Also, this was written before I dug into the actual CP code and now that I’m more comfortable with that, making some changes shouldn’t be a problem.
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.
So far in my testing, only a few displays needed to be initialized as BGR. The only driver that was complicated though was this one. This particular parameter is a little messy since it's also combined with the display rotation. There's 3 ways of doing this that I can see:
Personally my favorite option is #1 because the change is only in the driver. #2 is my second favorite, but I'm not sure if it will always work. #3 seems a bit sketchy and doesn't provide much benefit over #1, and is less flexible.
Another idea is we could always start with option #1 and implement option #2 after 4.0 stable is released.
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.
I like 1 but want to avoid MADCTL for rotation because it only impacts the memory access orientation, not screen refresh. If we always allow displayio to rotate "in software" rather than using MADCTL we can ensure our memory updates align with screen updates. This also means we can know in what direction the hardware scrolling of the display occurs.
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.
I agree, which is why I’m only using MADCTL for BGR vs RGB and leaving rotation at default.