Adafruit-ST7735-Library icon indicating copy to clipboard operation
Adafruit-ST7735-Library copied to clipboard

Added support for WaveShare-like modules (greentab offsets with blacktab colors-yellowtab?)

Open theloukou opened this issue 3 years ago • 16 comments

Added simple definition for WaveShare version module. They use GREENTAB offsets, but BLACKTAB colors (someone referred to them as YELLOWTAB?) Random cheapo modules seem to use this scheme as well.

*Initialize display with INITR_GREENTAB_WS, simple as that

theloukou avatar Mar 17 '22 15:03 theloukou

Just a note, the Waveshare module linked in the PR shows wrong colors and has correct offsets with the existing GREENTAB init (the example reccomended one), and correct colors but has wrong offsets in BLACKTAB init. Please confirm if anyone has this module or random ebay ones.

theloukou avatar Mar 17 '22 16:03 theloukou

This should be a cleaner solution to issue #154 as well, than having to manually edit library files.

theloukou avatar Mar 21 '22 09:03 theloukou

Hey, is anyone even checking/reviewing these PRs anymore?

theloukou avatar Sep 05 '22 09:09 theloukou

You solution doesn't work! It doesn't work, it just duplicates the already created initialization #define INITR_GREENTAB_WS INITR_GREENTAB This way works: https://github.com/adafruit/Adafruit-ST7735-Library/issues/154#issue-1041004912

brightproject avatar Mar 16 '23 15:03 brightproject

Have you even checked all the changed files in the PR? There are more changed besides the "duplicated initialization". The comment you linked does the exact same thing, but you "mess up" the original intend of the initializer. Mine adds a new one, so you keep both.

theloukou avatar Mar 16 '23 17:03 theloukou

Yes, I made changes in the Adafruit_ST7735.cpp and Adafruit_ST7735.h files as you indicated. Why doesn't the library developer change the code in his archive?

brightproject avatar Mar 16 '23 20:03 brightproject

Beats me, I am not the one responsible about merging this. You can just clone my forked repo btw instead of manually changing the files. Cheers!

theloukou avatar Mar 16 '23 22:03 theloukou

I believe your code is incorrect. By using #define INITR_GREENTAB_WS INITR_GREENTAB, you make INITR_GREENTAB indistinguishable from INITR_GREENTAB_WS, so even if your code works, it will break support for normal green tab displays.

samo-sk avatar Aug 10 '23 08:08 samo-sk

I believe your code is incorrect. By using #define INITR_GREENTAB_WS INITR_GREENTAB, you make INITR_GREENTAB indistinguishable from INITR_GREENTAB_WS, so even if your code works, it will break support for normal green tab displays.

Nope. You need to brush up your C stuff. Defining something does not work backwards. Code works fine. IF you test it and it doesn't, then come back and post your results with data.
Either way, it doesn't really matter, since noone from Adafruit seems to check this stuff since a loooong time.

theloukou avatar Aug 10 '23 18:08 theloukou

I only own a display that functions the same as the one described by you in the description of this pull request, so I can't test your changes directly. I can however test them indirectly. Using the original library with blacktab init causes the output to be shifted and with correct colors. Using the original library and greentab init causes the output to be not shifted and with incorrect colors. Using INITR_GREENTAB_WS with your library works OK with my display – no shift, correct colors. Output with blacktab doesn't change. However, output using greentab init is different from original – it is the same as when is used INITR_GREENTAB_WS. This is incorrect – you didn't want to modify the behavior of initR with greentab. PS: Compiling and running this code (on PC, not on Arduino) prints Equals., which disproves your argument on define:

#include <stdio.h>

#define INITR_GREENTAB 0x00 #define INITR_GREENTAB_WS INITR_GREENTAB

int testVar = INITR_GREENTAB_WS; int main() { if(testVar == INITR_GREENTAB) { puts("Equals."); } return 0; }

samo-sk avatar Aug 11 '23 09:08 samo-sk

You still don't get the point of my code. Your piece of code runs correctly, no problems there, but it's irellevant. IF you actually look at BOTH the files in this PR, the changes are not only the added definition. INIT_GREENTAB_WS uses some parts of INIT_GREENTAB code, and some parts of INIT_BLACKTAB code. The original functionality of INIT_GREENTAB is NOT affected, because it does not "back-define" INIT_GREENTAB_WS (which is the factor used in the added checks in the .cpp file.)

If this answer still does not satisfy you, consider this: the original .h file contains these lines

#define INITR_18GREENTAB INITR_GREENTAB
#define INITR_18REDTAB INITR_REDTAB
#define INITR_18BLACKTAB INITR_BLACKTAB

By your logic, defining any of these "18" variants should also mess the plain initializers. If you actually believe this, you should start wondering what black magic Adafruit does, and the original codes works. I'm over and out ☮️

theloukou avatar Aug 11 '23 10:08 theloukou

Please let me clarify. I know that you modified both of the files. I have copied your changes of both of the files into my copy of the library and I have tested your changes on a real Arduino and a real display – the comment above wasn't just a mere theoretical speculation. The displayed image when greentab was used was different from when the original library was used. You asked me not to argue with you on your code, so I won't do it. I just wanted to clarify my previous comment.

samo-sk avatar Aug 11 '23 10:08 samo-sk

Hi @theloukou. I am reading this thread. Do you think the lib would be compatible with this waveshare display? Thanks.

mchacher avatar Oct 29 '23 06:10 mchacher

Hi @theloukou. I am reading this thread. Do you think the lib would be compatible with this waveshare display? Thanks.

Your display uses a completely different driver from this library, so.... No!

theloukou avatar Oct 29 '23 13:10 theloukou

rather than adding a new #define which probably will change and requires maintenance, better to just add a comment to the example letting people know they can use BLACKTAB

ladyada avatar Oct 29 '23 16:10 ladyada

rather than adding a new #define which probably will change and requires maintenance, better to just add a comment to the example letting people know they can use BLACKTAB

The BLACKTAB define has wrong offsets with this screen, correct ones are GREENTAB ones. Hence why i made a new #define. Please check https://github.com/adafruit/Adafruit-ST7735-Library/pull/168#issuecomment-1071022756 if I was not understood.

theloukou avatar Oct 29 '23 16:10 theloukou