search

adafruit / circuitpython

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

Displayio column & row commands are 16 bit (need 8 bit too) #1683

Closed rdagger closed 5 years ago

rdagger commented 5 years ago

I wrote a CircuitPython displayio driver for the SSD1351 OLED 128x128 display. I confirmed the init sequence works by manually sending SPI commands to the display using the FourWire.send() method. However, none of the displayio commands work.

A closer look with a scope shows that displayio sends 16 bit values when it calls _setcolumn and _setrow. The SSD1351 expects 8 bit values for column and row.

Would it be possible to add a command_byte_length parameter to the displayio.display class constructor? It could default to 2 for 16 bit displays such as the ILI9341 and the user could pass 1 for 8 bit displays like the SSD1351. Scope SPI Compare01

makermelissa commented 5 years ago

@rdagger I noticed there's a comment in CircuitPython with this issue with a "To Do" next to it, so let's leave this open. However, I was able to get a working driver without this change for the SSD1351. As of right now the PR hasn't been merged yet, but you can download it off my repo until it has.

rdagger commented 5 years ago

@makermelissa I tested the SSD1351 driver and the example that you linked above and it doesn't work yet. I don't see how it could until the changes to _setcolumn & _setrow byte length are added?

Maybe I'm misunderstanding your post or maybe you have a different revision of the SSD1351? I'm using a generic SSD1351, but the Adafruit SSD1351 datasheet also shows a single byte for column/row start and a single byte for column/row end (page 36, section 10.1.1 & 10.1.2).

dhalbert commented 5 years ago

Could be 4.0.0 milestone or could be 4.x, depending on whether this appears to be necessary.

makermelissa commented 5 years ago

Thanks for testing @rdagger. It must have been the CircuitPython PR I prepared for this. I'll test it with the PR again.

makermelissa commented 5 years ago

Oh, I didn't address your comment @dhalbert. I'd say it should remain a milestone since it's necessary for displayio to work on certain displays.

@rdagger I swear it was working last night, but it's not working for me anymore, so now I'm trying to figure out what I did so I can duplicate. :)

rdagger commented 5 years ago

Seems like it will be difficult for the displayio.display class to maintain compatibility across all the different LCD displays. I took a look at writing a driver for an older Adafruit ST7565 display. Instead of a _setcolumn command it has _set_columnupper and _set_columnlower commands. Instead of _setrow it has _setpage (The display is broken into 8 horizontal pages which are numbered from the center 0 to 3 and 7 to 4).

deshipu commented 5 years ago

Fortunately there is a very limited number of different displays in use in there, and all the new ones follow the MIPI standard, so we could either hard-code the weird ones or simply completely leave them out.

makermelissa commented 5 years ago

In certain cases such as this display, I think there is enough flexibility to get it working. I had it displaying the other night, but must not have unplugged the display before testing Beta 5, so something was still properly initialized when I tested. I do know it works under the right conditions though.

makermelissa commented 5 years ago

Ok, I figured it out. I was using && instead of & for bitwise and. Once I fixed that it worked beautifully. Submitted PR so hopefully we'll get that up and running pretty soon.