Added the ST7735 Display Driver #2

makermelissa · 2019-03-21T06:45:55Z

I have subclassed all of the ST7735 variations and this fixes #1. There will be a separate PR for the ST7789.

makermelissa · 2019-03-21T06:48:03Z

Oh yeah, I fixed all the stuff so it passes on Travis too.

tannewt

Thank you for working on this! I made a number of comments because the Arduino driver isn't structured very well. I think we can add clarity to this driver and simplify if for everyone with a little more work.

CODE_OF_CONDUCT.md

tannewt · 2019-03-21T19:27:17Z

adafruit_st7735.py

+
+_INIT_R4_BLACK = (
+    b"\x36\x01\xC0" # _MADCTL Rotate to Landscape Mode
+)


Having all of these bytearrays in one file will take more memory that strictly necessary. I'd rather have multiple files with some duplication.

Ok, that makes sense.

tannewt · 2019-03-21T19:30:55Z

adafruit_st7735.py

    def __init__(self, bus, *, width, height):
-        super().__init__(bus, _INIT_SEQUENCE, width=width, height=height, colstart=2)
+        init_sequence = _INIT_R1 + _INIT_R2_GREEN + _INIT_R3


Adding all of the init sequences together will briefly make a duplicate allocation. I don't think it is necessary.

Instead, we can have them as mutable bytearrays that we can patch in the constructor.

I’ll look into that. I’m unfamiliar with the term.

tannewt · 2019-03-21T19:36:21Z

adafruit_st7735.py

+_INIT_R2_GREEN_160X80 = (
+    b"\x2a\x00\x00\x00\x4F" # _CASET XSTART = 0, XEND = 79
+    b"\x2b\x00\x00\x00\x9F" # _RASET XSTART = 0, XEND = 159
+)


These _INIT_R2 aren't needed because CircuitPython will reset them before sending any data.

I’ll test without these and remove them if they work the same.

Ok, I tested it and now I'm getting what you're saying.

tannewt · 2019-03-21T19:41:38Z

adafruit_st7735.py

+    def __init__(self, bus, *, width, height):
+        super().__init__(bus, _INIT_SEQUENCE_B, width=width, height=height)
+
+class MINI160X80(displayio.Display):


I'd like to see a separate library for Adafruit display products because it's the board and LCD pair that defines displayed resolution. The ST7735 functionality is separate of that. Once that is removed only ST7735B and ST7735R will remain. Then, in the adafruit display repo we can be very explicit about the setup on a per product number ID rather than tab color.

I think I understand what you mean. Does that mean the rotation and color mode can still be passed after initialization?

makermelissa · 2019-03-22T05:41:25Z

I think you'll like how it is much better. I know I do.

deshipu · 2019-03-22T11:25:08Z

adafruit_st7735/st7735b.py

+    b"\xE0\x10\x09\x16\x09\x20\x21\x1B\x13\x19\x17\x15\x1E\x2B\x04\x05\x02\x0E" # _GMCTRP1 Gamma
+    b"\xE1\x90\x0B\x14\x08\x1E\x22\x1D\x18\x1E\x1B\x1A\x24\x2B\x06\x06\x02\x0F\x0A" # _GMCTRN1
+    b"\x2a\x00\x02\x00\x81" # _CASET
+    b"\x2b\x00\x02\x00\x81" # _RASET


I think we don't need to set the column and row, because that is going to be done by displayio every time it actually sends anything to the display (the Arduino driver needs it, because it only sets the column and row once at the beginning, and then sends the whole screen every time).

Good catch. Fixed.

jerryneedell · 2019-03-22T12:47:08Z

Should the example be updated to use TileGrid instead of Sprites and x,y instead of position?
https://github.com/adafruit/Adafruit_CircuitPython_ST7735/blob/master/examples/st7735_simpletest.py#L16
Or should that be a separate PR

makermelissa · 2019-03-22T13:45:34Z

@jerryneedell, I updated the example in this PR to use TileGrid. I just went ahead and updated it in the Readme as well.

makermelissa · 2019-03-23T01:08:35Z

Since I just got a TFT Mini Breakout, I wrote an example for that too.

tannewt

It's looking way better! Thank you for your hard work on this. I added one comment about the init kwarg inline.

How do you feel about splitting this into two libraries? One for the ST7735 and one for the ST7735R. They really are separate driver chips even though the names are almost the same.

tannewt · 2019-03-26T22:06:01Z

adafruit_st7735/st7735r.py

+    """ST7735 driver for ST7735R"""
+    def __init__(self, bus, *, init=None, **kwargs):
+        """
+        :param bytes init: (Optional) An extra init sequence to append (default=None)


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.

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.

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:

Add a BGR parameter to the init function in the driver itself that adds an init code with the default rotation but sets BGR if true and make use of the rotation option in the driver.

Make changes to the way the colors are converted if this flag is toggled when they are 565 encoded as BGR instead of RGB. I don't know if colors are always converted, so if they're not this may not work.

Attempt to run the command inside of CP with default rotation if BGR is passed. This option could get messy for non-MIPI displays.

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.

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.

I agree, which is why I’m only using MADCTL for BGR vs RGB and leaving rotation at default.

deshipu · 2019-04-01T18:41:22Z

Tested with a 160x128 display, works well.

tannewt · 2019-04-01T22:41:06Z

As we chatted about, let's split this into two repos because the ST7735 and ST7735R chips are separate ICs and datasheets. That way the driver structure for displays matches non-displays.

makermelissa · 2019-04-02T05:39:23Z

Hi @tannewt, should the new Repo be named Adafruit_CircuitPython_ST7735R then? I'm just wondering if this may be more confusing to users who aren't as familiar with the differences between the R and non-R. Before starting on this, I wouldn't have known the difference. I'm thinking we should get rid of the non-R code and just keep this repo with the R code (since that's what the displays use) so that it's closer to the Arduino setup. Also, if it sits better with you, we could either rename this repo to the R version or create a repo with the R on it and delete this one.

tannewt · 2019-04-02T17:32:58Z

I've created a new repo for the R variant: https://github.com/adafruit/Adafruit_CircuitPython_ST7735R

Since we may want to use this name in the future it's better to create a new repo. Renaming this one causes a redirect from here to the new name.

While I agree the naming is potentially confusing I think awareness of this detail is needed for all drivers named after the IC. I don't expect many customers to work at this level anyway.

We're moving to a product driver model where Adafruit customers will either grab example code from the product guide or use a library like FeatherWing that does it for them.

Thanks for sticking with me!

makermelissa · 2019-04-02T17:41:25Z

Thanks for making that. I’ll separate this out tonight and create an additional PR for the R version.

deshipu · 2019-04-02T17:43:19Z

The problem here is that there are really two components here — the IC, and the display itself. While the IC is the same in all those versions, the actual display used, and more importantly, the way they are wired differs. For most other displays this is not a problem, because you only have one "standard" wiring per display size (so you can infer options like RGB/BGR from the size), but with the ST7735 modules there is more variety for some reason.

I'm still unsure if we don't simply want to have a "variant" parameter for creating the display object.

tannewt · 2019-04-02T18:12:18Z

@deshipu I totally agree. I don't think this library should handle the wiring side except that it exposes constructor arguments to use. The modules themselves can have libraries or example code for those details. It doesn't belong in this library.

Enforcing this separation ensures we document the options well and make it easier to use the IC with other displays.

makermelissa · 2019-04-03T04:50:08Z

Ok, separation has been done. All of the R code is in the other Repo and is the preferred once since that's what the Adafruit Displays use. This is now the non-R ST7735 driver which is untested since I am not aware of any ST7735 (non-R) displays to test this on. In theory it should work. I'm not sure if we want to release this as is and if somebody does come across a display that this is for and it doesn't work, test it then?

tannewt

Thanks again! You rule!

Melissa LeBlanc-Williams added 14 commits March 19, 2019 22:10

Subclassed all the displays. Not tested all yet.

8c4514b

Linting

382dbbb

More Linting

ca042ca

Bug fix from Linting

5a60f40

Hopefully a Fix for Travis and displayio

4ca2cea

Removing example for now

e49fd35

Attempting to fix cryptic Travis Error

06fc4a3

Removed Tutorial Link. Added Product Links

5cf39ce

Added Black Display

b9df192

Linting

040d4ea

Linting

29d10c8

Removed ST7789. Bug Fixes for MiniTFT.

39b75b9

Fixed pylint command

f4b6c8d

Final details

0fe8bda

makermelissa requested a review from a team March 21, 2019 06:46

Melissa LeBlanc-Williams added 2 commits March 21, 2019 00:03

Removed ST7789 Product Link

8690f94

Fixed example Bitmap Size

6921f75

tannewt requested changes Mar 21, 2019

View reviewed changes

Melissa LeBlanc-Williams added 2 commits March 21, 2019 22:36

Changes requested by tannewt

521dabe

Linting

e9c3636

makermelissa requested a review from tannewt March 22, 2019 05:40

deshipu reviewed Mar 22, 2019

View reviewed changes

Melissa LeBlanc-Williams added 2 commits March 22, 2019 06:42

Removing unnecessary sizing code

2c43ccb

Updated example in Readme

7107fdb

Melissa LeBlanc-Williams added 2 commits March 22, 2019 18:05

Added example for Mini TFT breakout

2cd7054

Removed unnecessary Reset Pin

1d3f15d

tannewt requested changes Mar 26, 2019

View reviewed changes

removed init byte code as an option

5662cb0

makermelissa force-pushed the master branch from c57ea97 to 5662cb0 Compare April 3, 2019 04:21

Separated Driver and Examples

b984037

makermelissa changed the title ~~Added all of the ST7735 Display Variations~~ Added the ST7735 Display Driver Apr 3, 2019

makermelissa requested a review from tannewt April 3, 2019 04:50

tannewt approved these changes Apr 3, 2019

View reviewed changes

tannewt merged commit 5f02c0d into adafruit:master Apr 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added the ST7735 Display Driver #2

Added the ST7735 Display Driver #2

makermelissa commented Mar 21, 2019

makermelissa commented Mar 21, 2019

tannewt left a comment

tannewt Mar 21, 2019

makermelissa Mar 21, 2019

tannewt Mar 21, 2019

makermelissa Mar 21, 2019

tannewt Mar 21, 2019

makermelissa Mar 21, 2019

makermelissa Mar 22, 2019

tannewt Mar 21, 2019

makermelissa Mar 21, 2019

makermelissa commented Mar 22, 2019

deshipu Mar 22, 2019

makermelissa Mar 22, 2019

jerryneedell commented Mar 22, 2019 •

edited

Loading

makermelissa commented Mar 22, 2019

makermelissa commented Mar 23, 2019

tannewt left a comment

tannewt Mar 26, 2019

makermelissa Mar 26, 2019

makermelissa Mar 28, 2019

tannewt Apr 1, 2019

makermelissa Apr 1, 2019

deshipu commented Apr 1, 2019

tannewt commented Apr 1, 2019

makermelissa commented Apr 2, 2019

tannewt commented Apr 2, 2019

makermelissa commented Apr 2, 2019

deshipu commented Apr 2, 2019

tannewt commented Apr 2, 2019

makermelissa commented Apr 3, 2019

tannewt left a comment

Added the ST7735 Display Driver #2

Added the ST7735 Display Driver #2

Conversation

makermelissa commented Mar 21, 2019

makermelissa commented Mar 21, 2019

tannewt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

makermelissa commented Mar 22, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerryneedell commented Mar 22, 2019 • edited Loading

makermelissa commented Mar 22, 2019

makermelissa commented Mar 23, 2019

tannewt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deshipu commented Apr 1, 2019

tannewt commented Apr 1, 2019

makermelissa commented Apr 2, 2019

tannewt commented Apr 2, 2019

makermelissa commented Apr 2, 2019

deshipu commented Apr 2, 2019

tannewt commented Apr 2, 2019

makermelissa commented Apr 3, 2019

tannewt left a comment

Choose a reason for hiding this comment

jerryneedell commented Mar 22, 2019 •

edited

Loading