bigtreetech / BIGTREETECH-TouchScreenFirmware

support TFT35 V1.0/V1.1/V1.2/V2.0/V3.0, TFT28, TFT24 V1.1, TFT43, TFT50, TFT70
GNU General Public License v3.0
1.32k stars 1.65k forks source link

[BUG] Preheat: name can causes the menu to freeze requiring a restart. #2898

Closed enewman17 closed 9 months ago

enewman17 commented 9 months ago

Using SKR mini E3 v3 with TFT35 E3 v3.0.1

BIGTREE_GD_TFT35_V3.0_E3.27.x.bin and https://github.com/MarlinFirmware/Marlin Board firmware built from source Marlin bugfix 2.1.x

Simple bug because of space limitations across different screens sizes. A description addition to the config.ini could clear up issues with the naming scheme. (Example below) or just changing the max characters to 8 (or perhaps 10 needs further testing) but 8 characters fit nicely in the preheat button area on the TFT35 E3 v3.0.1

#### Preheat Temperatures
#   Format: [preheat_name_X:<name>] (No Spaces)
#                [preheat_temp_X:T<hotend temp> B<bed temp>]
#                Unit: [temperature in °C]
#
####  Naming Scheme Max Characters     
#   [ *Screens that can handle the 20 characters here]    [min: 3, max: 20 characters] (No Spaces)
#   [TFT35 E3 v3.0.1 *And any other touch screen with limitations to button size]  [min: 3, max: 8 characters] (No Spaces)
#
#  Value range:            
#                hot end temp: [min: 20, max: 1000] 
#                bed temp:    [min: 20, max: 400]

Examples: 1) Adding a name with anything more than about 8 letters will not fit properly within the space of the preheat button. TFT35 E3 v3.0.1

2) Adding a "Space" in the name will cause the menu to freeze with weird characters to appear across the screen. Example:

preheat_name_6:Carbon PC
preheat_temp_6:T250 B90

Name without a space will cause no issues.

preheat_name_6:CarbonPC
preheat_temp_6:T250 B90

3) Adding a long name like (Example below) will cause the menu to freeze with weird characters to appear across the screen. Config.ini says [min: 3, max: 20 characters] perhaps this isn't an issue on larger screens or in marlin mode, but on the TFT35 E3 v3.0.1 in touch mode, this will cause issues. I did not fully test the amount of characters when issues start but it seems 8 characters is the max to fit the button. Polycarbonate [13 characters] definitely causes the menu to freeze requiring a reset.

preheat_name_6:Polycarbonate
preheat_temp_6:T250 B90
digant73 commented 9 months ago

in theory spaces and long names should be allowed and not causing any freeze but only a text overlapping in small TFT sizes such as TFT35 (e.g. a text longer than 8 chars will overlaps with other text etc...).. I see that a possible reason for the freeze is a missing data initialization in case the text is not in the range 3-20 but according to your description you have the freeze even with 13 chars or with the presence of a space. I will try to replicate the issue when possible.

digant73 commented 9 months ago

@enewman17 if you are on a recent TFT fw you could install the attached version without any need to update also the config.ini file. The attached fw should fix the issue (it was due to the text outside the background icon when the text was wider than the icon). With the fix, the text outside the icon boundary is not displayed

BIGTREE_GD_TFT35_V3.0_E3.27.x.zip

enewman17 commented 9 months ago

@digant73 Thank you. I will test it.

enewman17 commented 9 months ago

@digant73 I tested the firmware, it fixes the issue of freezing with longer names and spaces while creating a new issue. The name is now centered in the button layout so it cuts off the beginning and end of the words. I'm not a programmer but I have a basic understanding of how it works and have been building marlin for many years now. Personally I think in the case of these touch screens the word should probably just be limited to a smaller character count. With the fix you created the character count needs to be limited to 6 characters so its not cut off or the box size needs to be expanded to hold at least 8 characters. Thank you for taking the time to work on this.

I just got this card last week and so far I've also had the screen freeze after saving data from MPC tuning, in a few cases after flashing the screen firmware the homing directions of the steppers do whatever they want to do. That usually resolved itself after flashing the config.ini again or after a few power cycles, or unplugging the stepper. I'm not 100% sure if I have the Neopixels setup correctly for this display but they are not working for me from the touch menu. They do however work in Marlin mode, if they are turned on in Marlin mode or by gcode the touch menu will turn them off it just won't turn them on, not sure what is going on there. They are also not triggered by any events like preheating. I haven't printed anything yet so I don't know if Marlin is trigger any events either, I do know the setting is on though. I will get log files from the printer and take some pictures and post them with any other things I find.

digant73 commented 9 months ago

the text on Preheat menu is centered on the icon. To avoid the freeze, the displayed text must not be outside the icon (each character of the text must be fully inside the icon). For that reason if the text is wider than the icon width some leading and trailing characters are not displayed (it means that part of the character is outside the icon). There is no other better alternative (I could use another solution but the result is worse; pressing on the icon the text inside the icon disappears, like on the current solution, while the text outside the icon will remain displayed).

Never used MPC menu (not even configured on Marlin) so I cannot test it. For the LED strip:

  1. LED Event feature on TFT (selectable also from the Feature menu) is applied only during printing and requires Emulated M109_M190 feature (selectable also from the Feature menu) disabled. Read also the descriptions reported for those two settings on config.ini file
  2. I suspect that LED strip handling is now broken on Marlin side. It seems that the I<xx> option on M150 command is not working properly. The Custom LED menu uses the command: "M150 R U Bu W Pu Iu and the I option is used to select the specific LED to change. The menu allows to change each of those 6 param values. Value 255 for param I should be used to select all the LED but I verified that this value seems to corrupt something on Marlin replying (sometimes) with an Unknown Command message. It happens in particular when printing with advanced_ok and event_led features on TFT enabled and in particular at the beginning of the print when the LED event feature will periodically send the M150 commands (I255 is always used by LED event) to change the LED colors. If I don't use the I255 option (I changed the code) the issue is not present
enewman17 commented 9 months ago

Ahh... I understand the text and Icon situation, it's not a big deal.

I have the led event and emulated M109 on and I had a lot of issues with advanced ok turned on in the TFT menu. My understanding is since its enabled in Marlin the TFT option should be off. I also had a lot of checksum errors with the command checksum option turned on. I'll get logs on the MPC feature if it freezes again. Also from what I understand about the LED strips is the S parameter should be used to turn on or off the entire strip so M150 B255 S0 would turn Blue on the entire 1st strip at full brightness and M150 B0 S0 would turn it off and S1 would do so for a second strip. So I guess it would make sense that the param I would cause an issue.

Edit: M150 0 S0 or M150 0 will turn off the the entire strip no need to specify the color.

kisslorand commented 9 months ago

I had a lot of issues with advanced ok turned on in the TFT menu. I also had a lot of checksum errors with the command checksum option turned on.

For a stable and performant TFT you can try the FW from my repository.

digant73 commented 9 months ago

Ahh... I understand the text and Icon situation, it's not a big deal.

I have the led event and emulated M109 on and I had a lot of issues with advanced ok turned on in the TFT menu. My understanding is since its enabled in Marlin the TFT option should be off. I also had a lot of checksum errors with the command checksum option turned on. I'll get logs on the MPC feature if it freezes again.

You have to enable advanced ok on both Marlin and TFT. Even in this case read the description in config.ini for tx_slots, advanced_ok and command_checksum. Also, you can see the current available TX slots opening the Notification menu (press on the title bar on top of the display) and pressing on the button Info. You will see a screen reporting some current stats, one of them is the tx_slot (the number of concurrent gcode that can be sent to the mainboard). E.g. in my case 3 because I configured BUFSIZE to 4 in Marlin's Configuration_adv.h. If disabling advanced_ok in the TFT you still have checksum errors I suspect that they are real issues with the serial line (e.g. due to EMI). A snoop on the serial line or on Marlin side can clarify this issue.

Also from what I understand about the LED strips is the S parameter should be used to turn on or off the entire strip so M150 B255 S0 would turn Blue on the entire 1st strip at full brightness and M150 B0 S0 would turn it off and S1 would do so for a second strip. So I guess it would make sense that the param I would cause an issue.

Yes, I also saw this new option S but consider that it is currently not used on the current TFT fw. I will make some testing with it and eventually integrate on the TFT fw

rondlh commented 9 months ago

WARNING: Kisslorand's closed source firmware is not recommended, it's buggy, outdated and might damage your printer and/or display. His PRs here are not merged because they are buggy, slow and not peer reviewed, just have a look here: https://github.com/bigtreetech/BIGTREETECH-TouchScreenFirmware/pulls.

kisslorand commented 9 months ago

WARNING: There's a troll between us with ZERO contribution to this repository spreading false claims without any proof other than his rabid bark.

His hatred started when I refused to share with him the source code of how I made the FW for these TFTs more reliable than the one in this repository. The proof is all around in this repository how he praised my work until I declined his request to release the source code for the "Hesitation Guard" feature that I came up with and implemented. One must wonder why he wanted so badly the "buggy and slow" source code... He teamed up with Mr. Benito and came up with the "brilliant" idea to implement the deprecated ADVANCED_OK feature that absolutely not a single 3D printer manufacturer uses, no remote controller solution (ESP3D, Octoprint, Pronterface, etc) uses. No remote printing capable slicer uses that neither. Even thou he claims he can analyze someone else's code with a simple glance, he had to team up with somebody because he was plain simple not up to the task. There are a few unfinished and/or non-functional code attempts from him and one single pull request that hasn't been approved by the maintainers of this repository. He lives in the shadow of Mr. Benito, it's him who happily picks up the garbage he left and makes it functional.

That's the puppy of Mr. Benito spreading his "wisdom" from the safety of his master's shadow.

Go Sparky, go! Woof, woof! Make Il Duce happy! Woof, woof!

rondlh commented 9 months ago

I have the led event and emulated M109 on and I had a lot of issues with advanced ok turned on in the TFT menu. My understanding is since its enabled in Marlin the TFT option should be off. I also had a lot of checksum errors with the command checksum option turned on.

@enewman17 Only if advanced OK is enabled in Marlin then it will work for the TFT, otherwise the TFT will default back to sending commands without Advanced OK, which is fine. Advanced OK will put more stress on the serial connection, and Command CRC will show any serial data corruption that otherwise would not be noticed/reported. But both Advanced OK and Command CRC are not the cause of serial data corruption.

Here some tips that might help:

  1. Make sure that the Marlin RX buffer is large enough (RX_BUFFER_SIZE in Configuration_adv.h), I recommend 256 bytes or 512 is your hardware supports it.
  2. Use lower serial communication speeds (57600, 115200, 250000), BAUDRATE in Configuration.h
  3. If your hardware supports it (STM32F1-F4), enable SERIAL_DMA in Configuration_Adv.h, this solved my communication issues. (only present in recent Marlin betas).
  4. Keep your serial data wires as short as possible, and away from stepper driver cabling
digant73 commented 9 months ago

@enewman17 About the freeze on MPC process, can you confirm it happens when exiting from the menu (pressing on the back button)?

enewman17 commented 9 months ago

@enewman17 About the freeze on MPC process, can you confirm it happens when exiting from the menu (pressing on the back button)?

Yes

digant73 commented 9 months ago

@enewman17 ok thanks. The issue will be fixed on a next PR. The bug is currently affecting also LED Event feature. Currently, to avoid the issue, it should be enough (not tested yet) to disable Event LED and LCD Dimming feature (set both event_led and lcd_idle_time to 0 on config.ini file)

github-actions[bot] commented 6 months ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.