search

Xinyuan-LilyGO / T-Display-S3

MIT License
782 stars 195 forks source link

"Tips" section of READMe is not understandable #48

Closed teastainGit closed 1 year ago

teastainGit commented 1 year ago

Hi! I'm sorry to say this, but as an experienced Arduino maker, the "Tips" section of READMe is not understandable. Firstly when the board is first plugged in there is a grey screen saying connection to Xinyuan... ... ... what does this do? Second the Tips are more confusing than helpful. Once set up, the board behaves like a normal Arduino or ES32! I really like the product but the new users deserve more support. (Also I started my own GitHub repo for new users of T-Display S3 here: https://github.com/teastainGit/LilyGO-T-display-S3-setup-and-examples)

Everyone is free to help improve this resource! -Terry

bratoff commented 1 year ago

My bigger beef is that the directions have you (potentially) replace several standard Arduino libraries with the particular version Lilygo's developer used, which in many cases is much older than the current version. In most cases, the Tips should just have you make sure the latest versions are installed, using the Arduino Library Manager.

The unpleasant exception I have found is that there have been nonstandard modifications made to the TFT_eSPI library, which is also used by many other Lilygo boards. All you should ever have to do to configure TFT_eSPI is uncomment the correct setup file in User_Setup_Select.h, or possibly create a new Setupnnn_xxxxxx.h file in the User_Setups folder and add a line for it in User_Setup.h if a suitable setup does not already exist. This technique works for just about every other Lilygo board, but there appear to have been additional code changes made to the TFT_eSPI library in the T-DisplayS3 repository. This is unacceptable, as it creates conflicts when you're developing for multiple board types and makes it impossible to stay current with the latest official copy of the library. If these changes are really necessary and not just the result of a lack of understanding, Lilygo should make them on the latest official version of TFT_eSPI and then submit them as a PR to the library's maintainer.

mmMicky commented 1 year ago

My bigger beef is that the directions have you (potentially) replace several standard Arduino libraries with the particular version Lilygo's developer used, which in many cases is much older than the current version. In most cases, the Tips should just have you make sure the latest versions are installed, using the Arduino Library Manager.

The unpleasant exception I have found is that there have been nonstandard modifications made to the TFT_eSPI library, which is also used by many other Lilygo boards. All you should ever have to do to configure TFT_eSPI is uncomment the correct setup file in User_Setup_Select.h, or possibly create a new Setupnnn_xxxxxx.h file in the User_Setups folder and add a line for it in User_Setup.h if a suitable setup does not already exist. This technique works for just about every other Lilygo board, but there appear to have been additional code changes made to the TFT_eSPI library in the T-DisplayS3 repository. This is unacceptable, as it creates conflicts when you're developing for multiple board types and makes it impossible to stay current with the latest official copy of the library. If these changes are really necessary and not just the result of a lack of understanding, Lilygo should make them on the latest official version of TFT_eSPI and then submit them as a PR to the library's maintainer.

The I8080 interface of the TFT_eSPI library to the ESP32S3 chip is driven by software simulation. The library itself uses register operations for speed. In fact, the GPIO register width of ESP32S3 is only 32 bits, and one register is not enough to allocate more than 40 pins. So the gpio control register of ESP32S3 has two groups. In TFT_eSPI, it only takes effect on the low pin registers. So I had to modify the driver inside the library to support high pins.

mmMicky commented 1 year ago

At the same time, I will place the modified places in the TFT_eSPI directory in the form of patches. You can use this patch to supplement the new version TFT_eSPI.

mmMicky commented 1 year ago

https://github.com/Xinyuan-LilyGO/T-Display-S3/blob/main/lib/TFT_eSPI/micky_commit.patch

bratoff commented 1 year ago

https://github.com/Xinyuan-LilyGO/T-Display-S3/blob/main/lib/TFT_eSPI/micky_commit.patch

Thank you - I will apply the patches to the latest official TFT_eSPI and see how that works.

If it checks out, why not submit it as a pull request to the author of TFT_eSPI?

bratoff commented 1 year ago

https://github.com/Xinyuan-LilyGO/T-Display-S3/blob/main/lib/TFT_eSPI/micky_commit.patch

Thank you - I will apply the patches to the latest official TFT_eSPI and see how that works.

If it checks out, why not submit it as a pull request to the author of TFT_eSPI?

I have successfully merged Micky's patch with the latest official TFT_eSPI (version 2.4.79). Several differences from Micky's patch were necessary:

Here is a zip of my final patch. It is no longer necessary to completely overwrite the official library in your libraries folder. Instead, these files can just be copied on top of the official version 2.4.79 with no other changes necessary:

TFT_eSPI - T-DisplayS3 patch.zip

My patch files also include updated User_SetupNNN files for the original T-DIsplay and the T-Display-RP2040. If you have either of these boards, you only need to add these two files to the User_Setups folder and update User_Setup_Select.h in the official library.

dapug commented 1 year ago

Shouldn't we work with the TFT_eSPI author to make changes for this board? It would not affect other existing users of TFT_eSPI, right? It only takes effect with TFT_DATA_PIN_OFFSET_EN build_flags, correct?

The problem I am having is, I have a project using source I didnt write, and it dynamically is pulling the latest TFT_eSPI from the web, and I cannot seem to "patch" anything locally to take effect. Even if I could... this patch should be in the official lib, otherwise, to use t-display-s3 is very "hacky".

bratoff commented 1 year ago

Shouldn't we work with the TFT_eSPI author to make changes for this board? It would not affect other existing users of TFT_eSPI, right? It only takes effect with TFT_DATA_PIN_OFFSET_EN build_flags, correct?

The problem I am having is, I have a project using source I didnt write, and it dynamically is pulling the latest TFT_eSPI from the web, and I cannot seem to "patch" anything locally to take effect. Even if I could... this patch should be in the official lib, otherwise, to use t-display-s3 is very "hacky".

I agree. As Micky is the author of the patch and I jsut cleaned up a few things to make it work with the latest released TFT_eSPI, I was hoping Micky would make a pull request on the TFT_eSPI project, which would enable the TFT_eSPI author to merge the patch into a future release.

ElectricDream5 commented 1 year ago

Thank you Bratoff for posting this fix. I was pulling my hair for days after updating TFT_ESPI to version 4.79 on IDE 2.03. The screen is working once again.

I agree with OP TeaStainGit regarding the tips section needing improvements. They have added useful information in the past couple of months, but beginners like me could use a bit more hand holding during set up.Of course, this has to do with the TFT_ESP Lib adaptation for this board as well.

mmMicky commented 1 year ago

I have submitted a pull request to the TFT_eSPI author. https://github.com/Bodmer/TFT_eSPI/pull/2296

ElectricDream5 commented 1 year ago

Thank you for all the work you've done to make these great little boards more accessible.

ElectricDream5 commented 1 year ago

Hi Terry,

At that point in the thread I was responding to Mickey and thanking him/her for the work to make it easier to start up with the T Display S3. Of course, your point still stands, the instructions can be improved, so thanks for pointing that out. Well worth the effort.

Andres

From: @.> Sent: Wednesday, January 4, 2023 9:50 PM To: @.> Cc: @.>; @.> Subject: Re: [Xinyuan-LilyGO/T-Display-S3] "Tips" section of READMe is not understandable (Issue #48)

Hi Electric Dream, These issue boards are confusing! Who are you referring to with your thanks? I just need to know if my repository is helpful.

Cheers ! Terry

On Jan 4, 2023, at 8:57 PM, ElectricDream5 @.***> wrote:

 Thank you for all the work you've done to make these great little boards more accessible.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

— Reply to this email directly, view it on GitHubhttps://github.com/Xinyuan-LilyGO/T-Display-S3/issues/48#issuecomment-1371715830, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A5BKMXTM5KNBKYAIAW4ZWSTWQYZIRANCNFSM6AAAAAASCWPTOU. You are receiving this because you commented.Message ID: @.***>

Bodmer commented 1 year ago

The patch pull request has been applied to this TFT_eSPI master branch:

https://github.com/Bodmer/TFT_eSPI/tree/T-Display-S3

Please test before I merge into the master branch as I do not have one of these boards.

dapug commented 1 year ago

I have submitted a pull request to the TFT_eSPI author. Bodmer/TFT_eSPI#2296

Did that patch include updates that @bratoff made? And to @Bodmer 's question about testing... we can test if it works with the T-Display-S3 board, but how can we be sure other boards don't break?

mmMicky commented 1 year ago

The patch pull request has been applied to this TFT_eSPI master branch:

https://github.com/Bodmer/TFT_eSPI/tree/T-Display-S3

Please test before I merge into the master branch as I do not have one of these boards.

I used the library in this link with the alphaBlend_Test example and the example in the link below carry out testing.

https://github.com/Xinyuan-LilyGO/T-Display-S3/blob/main/example/tft/tft.ino#L21

works fine.

I suggest adding the following initialization commands to this model and using macros to control whether to enable or disable, sometimes it will make the screen display better.

https://github.com/Xinyuan-LilyGO/T-Display-S3/blob/f155cb3835dd8c9fadf6a65f4cbd613e26d881f9/example/tft/tft.ino#L21

Bodmer commented 1 year ago

The master library of TFT_eSPI has been updated to include support for 8 bit parallel bus using high (>31) GPIO numbers.

All display data bits (D0-7) must use either high (>=32) or low (<=31) processor GPIO numbers since a single register write is used to output all data bits at the same time.

svdrummer commented 1 year ago

Other than uncommenting line 131

include <User_Setups/Setup206_LilyGo_T_Display_S3.h>

is there anything else. The text is all over the pace in a T3

I downloaded the zip file from github as the Arduino version is old.

ElectricDream5 commented 1 year ago

Quick note to confirm the new TFT_ESPI library version 2.50 works nicely with T Display S3 using the new user selected profile ID 206 for 8 bit parallel display on Arduino IDE 2.03/Windows 10. Thank you Bodmer!

owiecc commented 1 year ago

I tried to run with TFT_ESPI v2.50 but I get a garbled or offset display. I uncommented #include <User_Setups/Setup206_LilyGo_T_Display_S3.h> in User_Setup_Select.h @ElectricDream5 did you do anything else?

Bodmer commented 1 year ago

Did you comment out the default setup line like this: //#include

owiecc commented 1 year ago

That helped, thanks.