Update icons, Fix ABL G-code sending on mode change, Fix text overlap in update screen, Move language translations to SPI flash to reduce main memory usage, Fix other Issues/Bugs

[guruathwal]

• Update icons.
• Fix ABL G-code sending on Mode change.
• Fix text overlap in update screen.
• Fix value step button not displaying the correct value in heat menu.
• Move language translations to SPI flash to reduce main memory usage and allow switching/updating language translations without recompiling the firmware. ([FR] IDEA: move language.c into language.ini #365)
  • To update/ add a second language, the user needs to copy the required language_xx.ini file from Language Packs folder to the SD card root and rename it to language.ini and press reset button.
• Fix click blank list item in the list widget menu.
• Prevent switching to printing screen when printing from marlin mode when SERIAL_ALWAYS_ON is enabled. (Bug when printing from Marlin mode. #1058)
• Fix wrong extruder count in case of mixing extruder.
• Add Encoder enable signal to enable/disable encoder on marlin firmware to prevent unexpected changes on the printer in touch mode. (Support for 'BTN_ENC_EN' pin in emulated DOGM MarlinFirmware/Marlin#19796)

Resolves #365, resolves #1058, resolves #1176

[Guilouz]

Why do you change some icons like eeprom_reset, eeprom_restore etc ? They are already very well like that...

[digant73]

@guruathwal I'm finishing the Mesh Editor but I see some overlaps of menus. I will attach some screenshots this evening.

The problem is that the Mesh Editor GUI is not the standard menu (with 8 icons). In the main loop of the mesh editor I use the loopBackEnd() function instead of the loopProcess(). This is due to avoid the notification bar/temperature bar on top of the display. However some information are still displayed on the top (such as the bullet on the right or the message "busy....).
Is there the possibility to fix that problems? I was thinking a possible solution is to register a callback to redraw a custom menu.
So the loopBackEnd() could display some info (such as notification popup or error or reminder popup) but then the loopBackEnd() will finally call the registered redraw callback to reprint the custom menu.

In your PR you mentioned you fixed text overlap. Is that fix a solution also for my problem with the custom menu?
I don't see any change in your PR about text overlap and ABL. Maybe you haven't committed all files yet

[traffic-light]

@digant73 he did fix the overlap in the messagescreen that tells the user he needs to update the fonts, icons and or config.ini

It's fixed in the code. It's now placing the fonts according to the font height ;-)

[digant73]

Below some pictures. In the first two you can see the problem. On top you can see the busy dot on the right and a notification in the center

[traffic-light]

@digant73

Looks good!
Can't you turn off those notification temporary to complete off?
That you set notification type to none?

[ETE-Design]

@digant73 Looks Great, when will you make a PR for it? Seems like you'r pretty good at this stuff... Could you mabye look at a GUI for Tramming Assistant also? :-D

[guruathwal]

@discip
Made some changes to ABL/UBL detection.
for Autopower, it looks like someone did not change the setting sign values after adding a new setting variable. resetting the settings should fix it.

@digant73 I would not recommend disabling the 'busy' icon and other similar system notifications.
As a rule of thumb, it is better to keep the title bar visible for a menu which required more than 10-15 seconds of user interaction.
In the pictures you posted, the grid size can be reduced as it is only for visual feedback, which will give more space for other controls. why are you using too small buttons condensed in a small area for a tiny display? You were not in favour of small buttons earlier? simplifying the UI by rearranging the elements and removing unnecessary ones will help solve the problem.

[discip]

@guruathwal
Thank you. 👍
I am going to test that later.

[digant73]

@guruathwal I agrre to allow displaying those kind of info (busy indicator, notification etc...). For that reason I asked you if it is possible to make some changes in the window handling to support redrawing of "custom" menu (a menu not using menuDrawPage() to redraw the menu) such as Numpad and now my mesh editor.
Even reducing the grid size (leaving the top part of the screen blank), the problem will not be fixed because it seems that only standard menus (using menuDrawPage()) are properly handled. For custom menu, currently it seems you can use only loopBackEnd() in the menu main loop. And even with the loopBackEnd() there are some artefacts (as in my screenshot) . My proposal was to change the window handling code in order loopProcess() (or the single loopBackEnd() and loopFrontEnd) can support also custom menu.
My custom menu is statefull, so you can force to reload even the entire menu (e.g. forcing a change in the current menu queue and then restoring the menu in the queue to reload it again).
Currently you should have the same artefact also in the "numpad" menu (try to open it and then from a terminal (e.g. from the laptop) try to send some gcodes that will trigger popups, error messages etc...) you should face some problems.

About the GUI, there is also a standard menu to fine tune the Z height of the selected item in the grid (that menu is opened clicking in the "value" box). The size of the buttons and text is pretty good IMHO. To solve the artefact problem, it should be enough that after an infiormation dispalyed on the top bar, or any displayed popup dialog, has to be cleared then a redraw of the current menu is performed. That redraw seems missing now.

EDIT: I also made some fixes on MBL menu etc... Those will be included in my next PR. So if you found some problems on MBL menu, no need to waste time on that

[digant73]

@digant73 Looks Great, when will you make a PR for it? Seems like you'r pretty good at this stuff... Could you mabye look at a GUI for Tramming Assistant also? :-D

it's under testing, Just making some optimizations and cleaning here and there. Some days and it should be available. However, the main problem is that the FLASH space for some TFT variants (all the STM32F2xxx) is reached (it's 256K - 32K). For that variants the compilation fails. The workaround is to change the limit to 256K - 28K (as it is on all the other variants).
Obviuosly this is only a way to compile the code not the final solution. Waiting for BTT fix for the reduction of the languages included in the binary.

hmm no, currently I don't have other work plans after this PR.

[digant73]

@discip
Made some changes to ABL/UBL detection.
for Autopower, it looks like someone did not change the setting sign values after adding a new setting variable. resetting the settings should fix it.

@guruathwal
Some problems could be related to initMachineSetting() and setupMachine() in Settings.c, The first is never invoked while the second is invoked in parseAck.c just in some cases (e.g. if FW is not Marlin) . Both of them should be invoked to initialize all the parameters to default values.
E.g. BL type is initialized in initMachineSetting and in setupMachine() (althought the problem of discip could be related to the ABL string returned by Marlin FW not matching with the ones in parseAck(). I saw you already changed those strings in parseAck().
Consider that I added a new parameter for "software endstops status" to "infoMachineSettings" and verified it was never initialize althought is correctly initialized initMachineSetting()

[discip]

@guruathwal
After having updated to your last commit, the ABL menu (icon) is only visible, if the bed is not yet calibrated.
As soon as the calibration is done, the menu disappears. (in Configuration.h this #define ENABLE_BL_VALUE 1 is set)

thanks
regards

[guruathwal]

@digant73 The changes you are suggesting are already available in the source

BIGTREETECH-TouchScreenFirmware/TFT/src/User/API/menu.c

Lines 278 to 286 in 4f2c665

	void setMenuType(MENU_TYPE type)
	{
	menuType = type;
	}
	MENU_TYPE getMenuType(void)
	{
	return menuType;
	}

.

Even reducing the grid size (leaving the top part of the screen blank), the problem will not be fixed because it seems that only standard menus (using menuDrawPage()) are properly handled.

You need to study the SendGcode.c and 'Numpad.c' file for implementing your custom menu and even the Printing.c for a mix of both.
why will you leave the title bar blank. You will have to show the title of the menu if it is the grid preview menu or point edit menu showing the index of the current point.

[digant73]

@guruathwal
The main loop in Numpad is:
while (1)

while in SendGcode and in the Mesh Editor the standard loop is used:

while(infoMenu.menu[infoMenu.cur] == menuSendGcode)

while (infoMenu.menu[infoMenu.cur] == menuMeshEditor)

giving the possibility to an external entity (e.g. loopProcess, loopBackEnd etc...) to put on top of the menu queue another menu forcing the Mesh Editor to temporary exit and giving the control to the new menu. The Mesh Editor menu should be resumed (reloaded) by the menu handler once the menu put on top of the queue is closed (or expired in case of a toast, busy indicator etc...) but this is not happening. It seems that not all info displayed in the top bar are based on the use of menu handler. As far as I remember, popups are based on menu queue but the info temporary displayed on the title bar area does not rely on that. All the temporary displayed info in the title bar should also rely on menu queue or it should be possible to register a redraw callback that will be called by all the modules not relying on menu queue. Once the info displayed by those modules (e.g. toast notify) has to be cleared, the registerd callback is invoked to redraw the underlying menu.
I don't see any function in menu.c to register a redraw callback. I see only a menuDrawTitle() function to draw the title bar area. But this is applicable only on standard menu

[guruathwal]

@digant73
The loop in numpad.c is used to avoid changing menu level. Did you not read the sendgcode.c file. It also does the same thing you want to acheive.
The full redraw of a regular menu is done with 'menuDrawPage()', if this is not called by your menu and you set the menu type to custom then it should not redraw at all.
The title bar area is used to display all sorts of information which are independent of the menu level and should not be used as regular menu area. If it is also done with the menu queue then the menu queue wiill keep switching between the titlebar and the actual menu triggering unnecessary screen redraws.

[digant73]

@guruathwal
yes, I understand the logic used in numpad (numpad is not a menu but a function requiring input and output attributes so it cannot be even managed by the queue handler. My menu is compliant to sendGcode (it is managed by the queue handler and use the loopBackEnd() to avoid the title bar info because they don't trigger a redrawn of the underlying menu.
But when the Mesh Editor menu is loaded by the queue manager, the artefacts in the pictures are present. They should not be drawn at all because I used loopBackEnd() in the main loop (just to avoid that kind of info). Once the menu is initially drawn (with that artefacts), no other info are displayed on the title bar (as I was expecting also at the menu startup because I use loopBackEnd() only in my menu main loop).
The artefacts info in my pictures are due to info related to gcodes sent in the main loop just before calling the loopBackEnd(). The loopBackEnd() sends the gcode and parse also the result provided by Marlin but, as you see in the pictures, notification and busy indicator are also drawn. Why did the loopBackEnd() draw that info if they should be handled only by loopFrontEnd() (that is not present in my menu)?

[traffic-light]

@guruathwal Good idea using a ini file for languages.

I don't know if anyone would agree, but I think it's easier, for the end user, to put every language.ini in a folder for the language and copy the one you need. (Instead of renaming the language_xx.ini into language.ini)

So example:

languages packs
            |______AM
            |       |_ language.ini
            |
            |______CA
            |       |_ language.ini
etc.

[Guilouz]

Possible to fix also M177 message in parceACK like in this post ?

#1050 (comment)

[digant73]

just some bugs and improvements.

bugs:

1. as I already explained, you now unified the level state with bl type using the unique variable infoMachineSettings.leveling.
   This simply doesn't work. At startup M115 is called after M503 and the parseAck() will always set infoMachineSettings.leveling to the value of Cap:AUTOLEVEL (that can assume only 0 and 1 as value). Also in case of MBL, Cap:AUTOLEVEL is set to 0. This means that in the best case you will see only the ABL menu (even if M503 provided UBL) or no BL menu at all (if MBL is enabled in Marlin). You should at least avoid to set infoMachineSettings.leveling inside Cap:AUTOLEVEL. But in that case you will simply restore the previous existing logic that was logically correct but (most probably for some bugs elsewhere) causing, for some users, the missing BL menu. It should be better to discover that bug.
2. At the end of the "Updating configuration" procedure (with the config.ini file), when the end of the process is reached (progress bar is at 100%) the system freezes. I had a look to the code and (in my case) it freezes on config.c at line 65. That means when "storePara();" is called.

improvements (to avoid possible causes of random bugs):

1. in settings.c some parameters were not initialized (such as rotate_ui whos deafult value is compared with the value from config.ini file (line 570 on config.c). This means a random handling calibration can be performed (on line 60 of config.c)
2. on config.ini file, on "Power Supply Settings" section, the parameter "ps_on_active_high" is defined but in config.c the value read form file is stored into the parameter infoSettings.powerloss_invert that doesn't have an equivalent setting in config.ini (it should be on "Power Loss Recovery" section). In config.c, it should be used "infoSettings.ps_active_high" instead of "infoSettings.powerloss_invert.powerloss_invert". This last parameter should be removed from settings.h and settings.c.
3. also, the list of paramters in settings.h and settings.c should be provided according to the sequence in config.ini (that is already aligned with config.c). This will make more easy a cross check in the project. The same should be made with flashStore.c (that is currently not aligned with config.c and config.ini. Attached I provided a reviewed version of settings.c and .h (including your last update on laguage paramter). It is aligned with the sequence in config.ini and config.c with the exception of all the arrays that are initialized at the end (in settings.c). I also provided the initialization for some parameters that were missing (e.g. rotate_ui).

settings.c_h.zip

EDIT: about the freeze during configuration update I verified that:

1. commenting line 65 (storePara()) there is no freeze and the config.ini file is loaded (obviously not stored) and the FW boots

[guruathwal]

1. as I already explained, you now unified the level state with bl type using the unique variable infoMachineSettings.leveling.
   This simply doesn't work. At startup M115 is called after M503 and the parseAck() will always set infoMachineSettings.leveling to the value of Cap:AUTOLEVEL (that can assume only 0 and 1 as value). Also in case of MBL, Cap:AUTOLEVEL is set to 0. This means that in the best case you will see only the ABL menu (even if M503 provided UBL) or no BL menu at all (if MBL is enabled in Marlin). You should at least avoid to set infoMachineSettings.leveling inside Cap:AUTOLEVEL. But in that case you will simply restore the previous existing logic that was logically correct but (most probably for some bugs elsewhere) causing, for some users, the missing BL menu. It should be better to discover that bug.

This can be fixed.

2. At the end of the "Updating configuration" procedure (with the config.ini file), when the end of the process is reached (progress bar is at 100%) the system freezes. I had a look to the code and (in my case) it freezes on config.c at line 65. That means when "storePara();" is called.

I am unable to reproduce this as it is working as expected in two different devices I have.

1. in settings.c some parameters were not initialized (such as rotate_ui whos deafult value is compared with the value from config.ini file (line 570 on config.c). This means a random handling calibration can be performed (on line 60 of config.c)

It is already initialized in flashStore.c and should never be initialized anywhere else. The config is read after loading the settings.

2. on config.ini file, on "Power Supply Settings" section, the parameter "ps_on_active_high" is defined but in config.c the value read form file is stored into the parameter infoSettings.powerloss_invert that doesn't have an equivalent setting in config.ini (it should be on "Power Loss Recovery" section). In config.c, it should be used "infoSettings.ps_active_high" instead of "infoSettings.powerloss_invert.powerloss_invert". This last parameter should be removed from settings.h and settings.c.

This feature is related to switching on/off the power supply/printer. It is independent of the Power Loss Recovery feature and should not be grouped with it. In the source the word invert make more sense to invert the logic of the said variable. Tha name in the config file is so to use easy to understand words as there are many users who does not know proper english.

[digant73]

1. as I already explained, you now unified the level state with bl type using the unique variable infoMachineSettings.leveling.
   This simply doesn't work. At startup M115 is called after M503 and the parseAck() will always set infoMachineSettings.leveling to the value of Cap:AUTOLEVEL (that can assume only 0 and 1 as value). Also in case of MBL, Cap:AUTOLEVEL is set to 0. This means that in the best case you will see only the ABL menu (even if M503 provided UBL) or no BL menu at all (if MBL is enabled in Marlin). You should at least avoid to set infoMachineSettings.leveling inside Cap:AUTOLEVEL. But in that case you will simply restore the previous existing logic that was logically correct but (most probably for some bugs elsewhere) causing, for some users, the missing BL menu. It should be better to discover that bug.

This can be fixed.
yes. I saw your easy fix (I solved the problem commenting out the entire row). However, the same solution was not working for someone although the correct strings were provided by M503. For someone the BL menu was never displayed. For other (e.g. discip) the BL menu disappeared after calibration. That should never happens because in parseAck() we set the bl type only to ABL, UBL and MBL (not to DISABLED or the previous UNKNOWN value)

1. At the end of the "Updating configuration" procedure (with the config.ini file), when the end of the process is reached (progress bar is at 100%) the system freezes. I had a look to the code and (in my case) it freezes on config.c at line 65. That means when "storePara();" is called.

I am unable to reproduce this as it is working as expected in two different devices I have.
Ok, unfortunately in my case it is sistematic. Maybe a temporization problem with other HW initialized or that needs to be initialized

1. in settings.c some parameters were not initialized (such as rotate_ui whos deafult value is compared with the value from config.ini file (line 570 on config.c). This means a random handling calibration can be performed (on line 60 of config.c)

It is already initialized in flashStore.c and should never be initialized anywhere else. The config is read after loading the settings.
Ok. I was thinking to the case were it was not yet present on the flash (a new parameter in the config.ini) and also not initialized in settings.c. I always provide a default value for all settings. They will then be replaced by the value stored in the flash, if any. The default value will be used in case the value in the flash or in the config.ini file is missing

1. on config.ini file, on "Power Supply Settings" section, the parameter "ps_on_active_high" is defined but in config.c the value read form file is stored into the parameter infoSettings.powerloss_invert that doesn't have an equivalent setting in config.ini (it should be on "Power Loss Recovery" section). In config.c, it should be used "infoSettings.ps_active_high" instead of "infoSettings.powerloss_invert.powerloss_invert". This last parameter should be removed from settings.h and settings.c.

This feature is related to switching on/off the power supply/printer. It is independent of the Power Loss Recovery feature and should not be grouped with it. In the source the word invert make more sense to invert the logic of the said variable. Tha name in the config file is so to use easy to understand words as there are many users who does not know proper english.

I see you merged my settings.c, .h. Just to align everything in the project, I would also align flashStore.c to Config and settings. Just a cleanup, not a bug.
Finally, if possible I would avoid to have so much languages resources in the project (currently there are 3 different language resources. 2 identical language packs, plus the built in language pack). I would provide only one language pack in the project
(that based on the new .ini files). I would provide the language pack at the same level of the theme packs

[radek8]

@guruathwal If you are already redesigning the system like this, take a look at setting the number of extruders. In the Configuration.h and config.ini file, I can set the number of extruders, but the parameter is overwritten by the detected value found from Marlin.
If I have a mixing hotend, Marlin reports only one extruder, and the value 2 I entered is overwritten to 1. In the current setting, manual setting of the number of extruders is not applicable.
I tentatively suggested patch #1123.
It would be better to correct the autodetection logic if 0 was entered in the number of extruders, then autodetection would be used, and if it was incorrectly detected, the user would be able to set his own value, which would take precedence over autodetection.

[radek8]

fix wrong extruder count for mixing hotend

This adjustment is not good.
If I have a two-color printer with one hotend and 2 extruders, with filament retraction, Marlin reports 2 extruders correctly, but after this adjustment, the TFT will incorrectly detect 2 hotends.

[guruathwal]

fix wrong extruder count for mixing hotend

This adjustment is not good.
If I have a two-color printer with one hotend and 2 extruders, with filament retraction, Marlin reports 2 extruders correctly, but after this adjustment, the TFT will incorrectly detect 2 hotends.

@radek8 I Iooked into the marlin firmware and the extruder count is actual extruder count setup up by the user and is not calculated/derived from anything. I do not have this type of setup, so I need more time to resolve it.

[radek8]

OK, leave it as it is. It's functional.
Only printers with a mixing hotend will enable the option in PR #1123

[radek8]

Compilation is an issue added by this PR

TFT\src\User\API\LCD_Encoder.c: In function 'HW_EncActiveSignalInit':
TFT\src\User\API\LCD_Encoder.c:28:5: warning: implicit declaration of function 'setEncActiveSignal'; did you mean 'HW_EncActiveSignalInit'? [-Wimpli
setEncActiveSignal(1);
^~~~~~~~~~~~~~~~~~
HW_EncActiveSignalInit
TFT\src\User\API\LCD_Encoder.c: At top level:
TFT\src\User\API\LCD_Encoder.c:31:8: warning: conflicting types for 'setEncActiveSignal'
void setEncActiveSignal(uint8_t status)
^~~~~~~~~~~~~~~~~~
TFT\src\User\API\LCD_Encoder.c:28:5: note: previous implicit declaration of 'setEncActiveSignal' was here
setEncActiveSignal(1);
^~~~~~~~~~~~~~~~~~

[Guilouz]

I have alert too but compiling is ok

[GregSKR]

Just wondering (maybe this is not important)...
In User/API/menu.c, lines 272, 486 and 511:
rapid_serial_comm()//perform backend printing loop between drawing icons to avoid printer idling
Isn't a semicolon missing after rapid_serial_comm()?
rapid_serial_comm(); //perform backend printing loop between drawing icons to avoid printer idling