Add detection of Mainboard firmware type( Marlin or Smoothieware), auto-detect LONG_FILENAME_HOST_SUPPORT, on-boardSD and serial_always_on option in config file & other bugfixes

[guruathwal]

• Add detection of Mainboard firmware type( Marlin or Smoothieware)
  Smoothieware does not support M115 Detailed reporting. Hence some settings need to be enabled or disabled in TFT for Smoothieware.
• Fix text display errors.
• Fix to prevent settings reset after config update.
• add Onboard SD card options for non-marlin firmware in the config file.
• add serial always On option in the config file.
• add missing Hotend count in the config file.
• Fix preheat temps. not updating in the config file.
• Fix fan max PWM update in the config file.
• Fix parameter settings not saving ([BUG] The setting parameter is not saved automatically in the EEPROM after editing #651).
• Remove all traces of EMERGENCY_PARSER.
• Fix Babystepping Z offset.
• Add auto-detect LONG_FILENAME_HOST_SUPPORT.
• Allow negative values in machine size (Bed Size only support positive dimensions.  #698)
• Fix broken pause/ resume gcode

Resolves #668 , resolves #651, resolves #626 , resolves #698

[digant73]

Hi

I merged your changes made in #663 with the master branch (you committed few days ago) and made a review of the code in particular I focused my attention to the parameters you made configurable by file and which constants (such as FAN_NUM, TOOL_NUM etc...) you replaced with the variable you load from file or SPI flash.
Attached my changes to your files. Below my explanation:

1. the infoSettings.send_cancel_gcode variable was not loaded from config.ini. I added the parameter to the config.ini file and load it on config.c file
2. I replaced the FAN_NUM, TOOL_NUM and EXT_NUM instances in all the project files with the infoSettings.fan_count, infoSettings.tool_count and infoSettings.ext_count respectively
3. I defined a MAX_FAN_COUNT constant set to 6 (same value as for the existing MAX_TOOL_COUNT etc...) and used it as size for data structures in the project related to fan info

Below some other comments about changes that should also be made on the project:

1. the constant HEATER_NUM (currently set to MAX_TOOL_COUNT + 1 = 7), defined as size on some array data structures, is often used in the project as the number of tools to iterate in a "for" instruction (e.g. for (TOOL i = NOZZLE0; i < HEATER_NUM; i++).... ). It should be better to iterate to (infoSettings.tool_num + 1) instead of HEATER_NUM otherwise you risk to provide info for tools not installed on the printer.
2. the start, end and cancel gcodes are not stored and read on/from SPI flash and seems they are not executed during printing. E.g. if I abort the print neither the cancel nor the end gcodes are executed. It seems that the structure "printcodes" on the printing.c file is not correctly initialized with the value from infoSettings.start_gcode, infoSettings.end_gcode and infoSettings.cancel_gcode

br

PR663_reviewed_files.zip

[guruathwal]

@digant73 Thanks for checking. Still, some of the replacements are missed. I will try to add these corrections as soon as possible.

[digant73]

@digant73 Thanks for checking. Still, some of the replacements are missed. I will try to add these corrections as soon as possible.

no problem.
If you want to provide also PR #691 please consider the attached file (and not the one provided in the previous zip file).
That file fixes the bug on displaying 6 tool temperatures instead of the ones for the configured tools

StatusScreen.zip

[guruathwal]

@digant73 already fixed it.. just pushing the commit. 👍

[SonBroku]

So far, all the problems I've been having are fixed with your branch now @guruathwal :)

For sure this needs to be merged into the master branch.

[extesy]

@bigtreetech can you merge please?

[Hukuma1]

Love not knowing anything about coding and reading your guys' progress on squashing the bugs. Thank you! :)

[digant73]

Hi

I checked all % specifiers in your last committed project (all the project, not just your changes) fixing all possible wrong data read on printf, storeCmd, mustStoreCmd etc.... In particular for all integer types with a size < 32bit (the native integer size on that HW) a casted the variable to (int). This will allow to convert even uint16 (so even uint8, int8, int16 etc...) without loosing their sign.
There were also just few cases were %f was used on an int type. I simply casted to (float) the integer variable. The logic followed by BTT is to prefer the use of int32 or uint32 just because it is the native size (32 bit) compliant to %d specifier. That will avoid them to pay attention on properly casting the variable when used on printf etc..
Fortunately the changes were not so much (just few files). Most of the changes are related to your variables. Please consider to integrate my changes asap, they will ensure at least your code is stable. Remenber to always check the impact in the project if you will have the need to change the type of your data structures (e.g. int to uint16_t. verify if you will need to apply casting due to usage of that variable on a printf etc....)

PR663_reviewed_files.zip

[digant73]

thanks for the changes. They are better than my suggestions (you changed the % specifier and used promotion).
Just a possible change on Numpad.c.
In the instruction

my_sprintf(tempstr, "%d", old_val);

old_val has type u32 but you used %d. Maybe %u was the desired specifier

[guruathwal]

@digant73
The my_sprintf / my_vsprintf does not have the %u specifier. You can see below from line no. 94.
It is converting all input values of any size (signed or unsigned) to a signed int with my_va_arg(p_next_arg, int); at line no. 102.

BIGTREETECH-TouchScreenFirmware/TFT/src/User/my_misc.c

Lines 67 to 166 in 56f9040

	int my_vsprintf(char *buf, const char *fmt, my_va_list args)
	{
	char* p;
	my_va_list p_next_arg = args;
	uint8_t bit_width[2] = {0, 6};
	uint8_t bit_sel = 0;
	for (p=buf; *fmt; fmt++)
	{
	if (*fmt != '%')
	{
	*p++ = *fmt;
	continue;
	}
	bit_width[0] = 0;
	bit_width[1] = 6;
	bit_sel = 0;
	repeat:
	fmt++;
	if (*fmt >= '0' && *fmt <= '9' && bit_sel < 2)
	{
	bit_width[bit_sel] = *fmt - '0';
	goto repeat;
	}
	switch (*fmt)
	{
	case 'd': //Decimal integer //十进制整数
	{
	int n = my_va_arg(p_next_arg, int);
	p += intToString(p, n, 10, 0);
	break;
	}
	case 'x': //Hexadecimal integer //十六进制整数
	{
	int n = my_va_arg(p_next_arg, int);
	p += intToString(p, n, 16, 0);
	break;
	}
	case 'f': //Floating point number //浮点数
	{
	if((unsigned long)p_next_arg & 0x7) //Variable parametersFloating point numbers are double by default.Ensure 8-byte memory alignment //可变参 浮点数默认是double类型 保证内存8字节对齐
	{
	p_next_arg = (my_va_list)((unsigned long)p_next_arg + 0x7);
	p_next_arg = (my_va_list)((unsigned long)p_next_arg & 0xFFFFFFF8);
	}
	double f = my_va_arg(p_next_arg, double); //%f, output floating point number //%f，输出浮点数
	int n = (int)f;
	p += intToString(p, n, 10, f < 0);
	*p++ = '.';
	double d = ABS(f - n) + 0.5/MIN(1000000, POW_10[bit_width[1]]);
	for(int i=0; i < MIN(6, bit_width[1]); i++)
	{
	d *= 10;
	*p++ = (((int)d) % 10) + '0';
	}
	break;
	}
	case 'c': //Single ASCII character //单个 ASCII 字符
	{
	*p++ = my_va_arg(p_next_arg, int);
	break;
	}
	case 's': //String //字符串
	{
	char *str = my_va_arg(p_next_arg, char *);
	for (; *str != 0; )
	{
	*p++ = *str++;
	}
	break;
	}
	case '%': //
	{
	*p++ = '%';
	break;
	}
	case '.':
	{
	bit_sel++;
	goto repeat;
	}
	default:
	{
	break;
	}
	}
	}
	*p++ = 0;
	return (p - buf);
	}
	void my_sprintf(char *buf, const char *fmt, ...)
	{
	my_va_list ap;
	my_va_start(ap, fmt);
	my_vsprintf(buf, fmt, ap);
	my_va_end(ap);
	}

[digant73]

@digant73
The my_sprintf / my_vsprintf does not have the %u specifier. You can see below from line no. 94.
It is converting all input values of any size (signed or unsigned) to a signed int with my_va_arg(p_next_arg, int); at line no. 102.

BIGTREETECH-TouchScreenFirmware/TFT/src/User/my_misc.c

Lines 67 to 166 in 56f9040

	int my_vsprintf(char *buf, const char *fmt, my_va_list args)
	{
	char* p;
	my_va_list p_next_arg = args;
	uint8_t bit_width[2] = {0, 6};
	uint8_t bit_sel = 0;
	for (p=buf; *fmt; fmt++)
	{
	if (*fmt != '%')
	{
	*p++ = *fmt;
	continue;
	}
	bit_width[0] = 0;
	bit_width[1] = 6;
	bit_sel = 0;
	repeat:
	fmt++;
	if (*fmt >= '0' && *fmt <= '9' && bit_sel < 2)
	{
	bit_width[bit_sel] = *fmt - '0';
	goto repeat;
	}
	switch (*fmt)
	{
	case 'd': //Decimal integer //十进制整数
	{
	int n = my_va_arg(p_next_arg, int);
	p += intToString(p, n, 10, 0);
	break;
	}
	case 'x': //Hexadecimal integer //十六进制整数
	{
	int n = my_va_arg(p_next_arg, int);
	p += intToString(p, n, 16, 0);
	break;
	}
	case 'f': //Floating point number //浮点数
	{
	if((unsigned long)p_next_arg & 0x7) //Variable parametersFloating point numbers are double by default.Ensure 8-byte memory alignment //可变参 浮点数默认是double类型 保证内存8字节对齐
	{
	p_next_arg = (my_va_list)((unsigned long)p_next_arg + 0x7);
	p_next_arg = (my_va_list)((unsigned long)p_next_arg & 0xFFFFFFF8);
	}
	double f = my_va_arg(p_next_arg, double); //%f, output floating point number //%f，输出浮点数
	int n = (int)f;
	p += intToString(p, n, 10, f < 0);
	*p++ = '.';
	double d = ABS(f - n) + 0.5/MIN(1000000, POW_10[bit_width[1]]);
	for(int i=0; i < MIN(6, bit_width[1]); i++)
	{
	d *= 10;
	*p++ = (((int)d) % 10) + '0';
	}
	break;
	}
	case 'c': //Single ASCII character //单个 ASCII 字符
	{
	*p++ = my_va_arg(p_next_arg, int);
	break;
	}
	case 's': //String //字符串
	{
	char *str = my_va_arg(p_next_arg, char *);
	for (; *str != 0; )
	{
	*p++ = *str++;
	}
	break;
	}
	case '%': //
	{
	*p++ = '%';
	break;
	}
	case '.':
	{
	bit_sel++;
	goto repeat;
	}
	default:
	{
	break;
	}
	}
	}
	*p++ = 0;
	return (p - buf);
	}
	void my_sprintf(char *buf, const char *fmt, ...)
	{
	my_va_list ap;
	my_va_start(ap, fmt);
	my_vsprintf(buf, fmt, ap);
	my_va_end(ap);
	}

right. The chinese comment in the code helped me a lot to understand!
Is there any list of possible reported bugs you are checking?

[carl1961]

google helps , but seem the English is before the Chinese comments (I saw this after I posted LOL) Express integers in character form For hexadecimal, 10 will be represented as A Put integer + '0' = ASCII code corresponding to integer So the result in str is upside down, flip it Decimal integer Hexadecimal integer Floating point String

…

On Fri, May 22, 2020 at 6:22 AM digant73 ***@***.***> wrote: @digant73 <https://github.com/digant73> The my_sprintf / my_vsprintf does not have the %u specifier. You can see below from line no. 94. It is converting all input values of any size (signed or unsigned) to a signed int with my_va_arg(p_next_arg, int); at line no. 102. https://github.com/bigtreetech/BIGTREETECH-TouchScreenFirmware/blob/56f904000f16f972a5a5ed3b5bfb121fac739023/TFT/src/User/my_misc.c#L67-L166 right. The chinese comment in the code helped me a lot to understand! Is there any list of possible reported bugs you are checking? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#663 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXKJNPUAGHE2M56DNGZ4SLRSZOAZANCNFSM4M367U5A> .

[digant73]

google helps , but seem the English is before the Chinese comments (I saw this after I posted LOL) Express integers in character form For hexadecimal, 10 will be represented as A Put integer + '0' = ASCII code corresponding to integer So the result in str is upside down, flip it Decimal integer Hexadecimal integer Floating point String
…
On Fri, May 22, 2020 at 6:22 AM digant73 @.***> wrote: @digant73 https://github.com/digant73 The my_sprintf / my_vsprintf does not have the %u specifier. You can see below from line no. 94. It is converting all input values of any size (signed or unsigned) to a signed int with my_va_arg(p_next_arg, int); at line no. 102.

BIGTREETECH-TouchScreenFirmware/TFT/src/User/my_misc.c

Lines 67 to 166 in 56f9040

	int my_vsprintf(char *buf, const char *fmt, my_va_list args)
	{
	char* p;
	my_va_list p_next_arg = args;
	uint8_t bit_width[2] = {0, 6};
	uint8_t bit_sel = 0;
	for (p=buf; *fmt; fmt++)
	{
	if (*fmt != '%')
	{
	*p++ = *fmt;
	continue;
	}
	bit_width[0] = 0;
	bit_width[1] = 6;
	bit_sel = 0;
	repeat:
	fmt++;
	if (*fmt >= '0' && *fmt <= '9' && bit_sel < 2)
	{
	bit_width[bit_sel] = *fmt - '0';
	goto repeat;
	}
	switch (*fmt)
	{
	case 'd': //Decimal integer //十进制整数
	{
	int n = my_va_arg(p_next_arg, int);
	p += intToString(p, n, 10, 0);
	break;
	}
	case 'x': //Hexadecimal integer //十六进制整数
	{
	int n = my_va_arg(p_next_arg, int);
	p += intToString(p, n, 16, 0);
	break;
	}
	case 'f': //Floating point number //浮点数
	{
	if((unsigned long)p_next_arg & 0x7) //Variable parametersFloating point numbers are double by default.Ensure 8-byte memory alignment //可变参 浮点数默认是double类型 保证内存8字节对齐
	{
	p_next_arg = (my_va_list)((unsigned long)p_next_arg + 0x7);
	p_next_arg = (my_va_list)((unsigned long)p_next_arg & 0xFFFFFFF8);
	}
	double f = my_va_arg(p_next_arg, double); //%f, output floating point number //%f，输出浮点数
	int n = (int)f;
	p += intToString(p, n, 10, f < 0);
	*p++ = '.';
	double d = ABS(f - n) + 0.5/MIN(1000000, POW_10[bit_width[1]]);
	for(int i=0; i < MIN(6, bit_width[1]); i++)
	{
	d *= 10;
	*p++ = (((int)d) % 10) + '0';
	}
	break;
	}
	case 'c': //Single ASCII character //单个 ASCII 字符
	{
	*p++ = my_va_arg(p_next_arg, int);
	break;
	}
	case 's': //String //字符串
	{
	char *str = my_va_arg(p_next_arg, char *);
	for (; *str != 0; )
	{
	*p++ = *str++;
	}
	break;
	}
	case '%': //
	{
	*p++ = '%';
	break;
	}
	case '.':
	{
	bit_sel++;
	goto repeat;
	}
	default:
	{
	break;
	}
	}
	}
	*p++ = 0;
	return (p - buf);
	}
	void my_sprintf(char *buf, const char *fmt, ...)
	{
	my_va_list ap;
	my_va_start(ap, fmt);
	my_vsprintf(buf, fmt, ap);
	my_va_end(ap);
	}

right. The chinese comment in the code helped me a lot to understand! Is there any list of possible reported bugs you are checking? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#663 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXKJNPUAGHE2M56DNGZ4SLRSZOAZANCNFSM4M367U5A .

I was joking about the comment in chinese

[digant73]

@guruathwal According to the bug reporting no save on EEPROM was made on exiting from ParameterSettings menu, I honestly don't like that a permanent change is made without the user is prompted with a question asking if he wants to proceed to make permanent the changes. Furthermore (as it was in my case) someone could change the settings just for tuning the printer and testing the results (e.g. the steps-per-unit change) before he wants really make them permanent.
I had already made (for other usage) two dialog box handling the eeprom save and reset and calling them when needed (on my project I added new menus and fixed existing ones such as BabyStep, ProbeOffset, PreheatMenu, UnifiedHeat).
The changes I made are on top of your PR, so please let me know if you agree to change the current logic to always save on eeprom without prompting any message. I will provide you the changes on top of your PR.

Just about the bugs and improvements (not related to your PR) I found and fixed in my project that could be integrated in your PR:

1. babystep menu:
   *) the range limit of -5.00 to 5.00 could be not reached changing the unit value. E.g. if step is currently set to 4.10 and you change the unit value to 1.00 you cannot increment the step (it will remain 4.10 instead of becoming 5.00)
   *) The most important bug is that if you have set step to a value > 3.00 or < -3.00 the reset button will not work properly. The maximun value you can increase or decreese to Z-offset is 3.00. E.g. starting from an initial value of 0.00 for step and 0.00 for Z-offset, if you set step to 5.00 (so Z-offset will be increased to 5.00) then when you press on the reset button, step will become 0.00 but z-offset will be 2.00 (a maximum of 3.00 was decreased from z-offset on I single gcode invocation). I found that bug because I changed the babystep menu displaying also the z-offset.
   Both bugs were fixed on my project
2. ProbeOffset menu:
   *) same bug on range limits as in babystep menu. Here the range limit is - 20 to +20 but you normally cannot reach that limits
   *) resetting the offset must also reset babystep. This is not done instead
   Both bugs were fixed on my project
3. Preheat menu:
   *) No button to stop pheheat is present.
   I added a stop button (using existing stop icon so no need to import new icon, label etc...) allowing to stop both, bed or nozzle preheat
4. UnifiedHeat menu:
   *) Cooldown button stops only the heaters and not also the fans (both Heat and Fan menus are present on that menu so the cooldown must be applied to both). It should stop both heaters and fans
   Now, when cooldown button is pressed both heaters and fans are stopped

All of those fixes/improvements are already on top of your PR, fully tested since several weeks.

Please let me know if you would consider to integrate that fixes/enhancements in your PR. I will eventually open a PR for that but this will take longer time to be eventually accepted and integrated in the master branch.

[radek8]

Can you please add the fixes for using the coder that I suggested in
#690

[discip]

@digant73
Good morning,
just as you might have expected, I am very curious about those fixes/ enhancements you posted about above, so is there any chance you would provide them?

thank you in advance
regards

[guruathwal]

@digant73 I do not have any list of known bugs. I try to fix problems I find while using these devices, or a reported issue on Github or if anyone asks to solve a problem.

@guruathwal According to the bug reporting no save on EEPROM was made on exiting from ParameterSettings menu, I honestly don't like that a permanent change is made without the user is prompted with a question asking if he wants to proceed to make permanent the changes. Furthermore (as it was in my case) someone could change the settings just for tuning the printer and testing the results (e.g. the steps-per-unit change) before he wants really make them permanent.

That's a nice idea. I never had this as I prefer saving the settings in case I forgot to save after printing or before shutting down / reset.

1. babystep menu:
   *) the range limit of -5.00 to 5.00 could be not reached changing the unit value. E.g. if step is currently set to 4.10 and you change the unit value to 1.00 you cannot increment the step (it will remain 4.10 instead of becoming 5.00)
   *) The most important bug is that if you have set step to a value > 3.00 or < -3.00 the reset button will not work properly. The maximun value you can increase or decreese to Z-offset is 3.00. E.g. starting from an initial value of 0.00 for step and 0.00 for Z-offset, if you set step to 5.00 (so Z-offset will be increased to 5.00) then when you press on the reset button, step will become 0.00 but z-offset will be 2.00 (a maximum of 3.00 was decreased from z-offset on I single gcode invocation). I found that bug because I changed the babystep menu displaying also the z-offset.
   Both bugs were fixed on my project
2. ProbeOffset menu:
   *) same bug on range limits as in babystep menu. Here the range limit is - 20 to +20 but you normally cannot reach that limits
   *) resetting the offset must also reset babystep. This is not done instead
   Both bugs were fixed on my project

In the baby-stepping menu, while printing, the max range I had to set is +-1mm, and I never used the probe offset menu. So I do not know much about this bug. If you can share the details and fix, it will be great.
Although I doubt if while printing baby-stepping more than +-2mm is needed? if more than this is needed then will it not be a bad probe offset value then?

3. Preheat menu:
   *) No button to stop pheheat is present.
   I added a stop button (using existing stop icon so no need to import new icon, label etc...) allowing to stop both, bed or nozzle preheat

I am integrating the preheat options in the heating menu, so there will be a stop button in heat menu.

4. UnifiedHeat menu:
   *) Cooldown button stops only the heaters and not also the fans (both Heat and Fan menus are present on that menu so the cooldown must be applied to both). It should stop both heaters and fans
   Now, when cooldown button is pressed both heaters and fans are stopped

I did not add fan stop command as few people suggested to keep fans ON to speed the cooling process. Maybe it will be better to add cooldown command to the config file.

Please let me know if you would consider to integrate that fixes/enhancements in your PR. I will eventually open a PR for that but this will take longer time to be eventually accepted and integrated in the master branch.

you can share the details or you can open a PR to my Mainstream branch, then it will be added to this PR after merging.

[GregSKR]

@guruathwal Thanks for your work! 👍
Looks like some of the changes you made in the code are reverted since BTT commit 46969db...
eg. for heat.c the use of 'HEATER_COUNT' has gone
Same for Config.ini (cleaning code) & Printing.c (lines 742-745 added are not yours)...

[digant73]

@digant73 I do not have any list of known bugs. I try to fix problems I find while using these devices, or a reported issue on Github or if anyone asks to solve a problem.

@guruathwal According to the bug reporting no save on EEPROM was made on exiting from ParameterSettings menu, I honestly don't like that a permanent change is made without the user is prompted with a question asking if he wants to proceed to make permanent the changes. Furthermore (as it was in my case) someone could change the settings just for tuning the printer and testing the results (e.g. the steps-per-unit change) before he wants really make them permanent.

That's a nice idea. I never had this as I prefer saving the settings in case I forgot to save after printing or before shutting down / reset.

Good. I added the two functions "menuEepromSave" and "menuEepromReset" on "MachineSettings.c" and "MachineSettings.h", so they can be called by any other file in the project. Inside these functions a check if EEPROM is enabled or not is performed before eventually showing the popup screen. In case EEPROM is not enabled on Marlin, no popup is showed at all and the control is immediately returned to the caller (it is absolutely transparent for the end user).
I also replaced any invocation of "M500" in the project with "menuEeepromSave". The affected files were "leveling.c" and the mentioned "Parametersettings.c". 3 new labels were added for showing the proper text on the popup dialog box. I provided only english translation for all the locales so in the future a proper translation should be done for any locale.
A small improvement was also applied on "MachineSettings.c" and "Parametersettings.c". In particular the "M503" command was moved from "menuMachineSettings" to "menuParameterSettings", so now even the update of the parameters is made internally on "menuParameterSettings" before the parameters are then displayed on the GUI

1. babystep menu:
   *) the range limit of -5.00 to 5.00 could be not reached changing the unit value. E.g. if step is currently set to 4.10 and you change the unit value to 1.00 you cannot increment the step (it will remain 4.10 instead of becoming 5.00)
   *) The most important bug is that if you have set step to a value > 3.00 or < -3.00 the reset button will not work properly. The maximun value you can increase or decreese to Z-offset is 3.00. E.g. starting from an initial value of 0.00 for step and 0.00 for Z-offset, if you set step to 5.00 (so Z-offset will be increased to 5.00) then when you press on the reset button, step will become 0.00 but z-offset will be 2.00 (a maximum of 3.00 was decreased from z-offset on I single gcode invocation). I found that bug because I changed the babystep menu displaying also the z-offset.
   Both bugs were fixed on my project
2. ProbeOffset menu:
   *) same bug on range limits as in babystep menu. Here the range limit is - 20 to +20 but you normally cannot reach that limits
   *) resetting the offset must also reset babystep. This is not done instead
   Both bugs were fixed on my project

In the baby-stepping menu, while printing, the max range I had to set is +-1mm, and I never used the probe offset menu. So I do not know much about this bug. If you can share the details and fix, it will be great.
Although I doubt if while printing baby-stepping more than +-2mm is needed? if more than this is needed then will it not be a bad probe offset value then?

Yes +-1.00 is the max unit you can use to increase or decrease the step value. But, for example, if you set the unit value to 0.10 and increase the step value to 4.40. After that you set the unit to 1.00 and try to increase the step value, the step value remains 4.40 instead of reaching the limit 5.00. That problem is probably present also on the heating menus but here the problem was pretty easy to be found due to the small range of admitted values.
I fixed this problem on both "BabyStep.c" and "ProbeOffset.c" (here the range is -20 +20 but the code is pretty identical to babystep). In order to not write much code on the "switch" statement I preferred to organize the the code in functions that are then called on each "case" of the "switch" statement (e.g a decOffset function is called when pressing on dec button etc...).
About the possibility of setting a step value > 2, I agree with you (it means that the levelling is totally wrong and it should be performed again). However -5.00 +5.00 is the range provided by BTT (i didn't change it).
In order to guarantee that the reset of a step value > 3 is properly handled, I call the M290 command with a maximun of 1.00 unit. E.g. if step value is 5.00, M290 is called 5 times. E.g. if step value was 4.40, M290 is called 4 times with a 1.00 unit value and 1 time with 0.40 unit value.

About ProbeOffset, in addition to the bug on value range already discussed for BabyStep, a more important bug was also discovered and solved. The bug consisted on not resetting to 0.00 the babystep value every time a change on Z offset value is performed on the ProbeOffset menu.
With the fix, every time you change a Z offset value, the babystep value is now reset to 0,
With the bug, the babystep value will not be aligned with the new z-offset once z offset is changed by ProbeOffset menu.
E.g. from BabyStep menu you set baby step value to 5.00 and Z-offset is also 5.00. Then from ProbeOffset menu you change Z-offset to 3.00.
With the fix, if you move again on the BabyStep menu you will see that the babystep value is now reset to 0.00 while Z-offset is the expect value 3.00.
With the bug (baby step not reset), you will have baby step set to 5.00 and Z offset set to 3.00. If you click on reset button you will have 0.00 as baby step value and -2.00 as Z offset (you will probably scratch the bed if you move the extruder).

1. Preheat menu:
   *) No button to stop pheheat is present.
   I added a stop button (using existing stop icon so no need to import new icon, label etc...) allowing to stop both, bed or nozzle preheat

I am integrating the preheat options in the heating menu, so there will be a stop button in heat menu.

Ok, However attached you can also find my improvements for "Fan.c" and "PreheatMenu.c".
On "Fan.c", In case only 1 fan is configured, I replaced the next fan selection button (useless because we have only one fan selectable) with a more usefull half speed button. The half speed icon is already present in the BTT project so there was no need for a new icon.
On "PreheatMenu.c", The stop button is applied on the selected tool (both, nozzle or bed) as it happens for the abs, pla and petg buttons. An easy way to rollback a previously pressed preheat button.
Feel free to provide those improvements on a next PR or to skip them. For me they were pretty usefull.

1. UnifiedHeat menu:
   *) Cooldown button stops only the heaters and not also the fans (both Heat and Fan menus are present on that menu so the cooldown must be applied to both). It should stop both heaters and fans
   Now, when cooldown button is pressed both heaters and fans are stopped

I did not add fan stop command as few people suggested to keep fans ON to speed the cooling process. Maybe it will be better to add cooldown command to the config file.

Please let me know if you would consider to integrate that fixes/enhancements in your PR. I will eventually open a PR for that but this will take longer time to be eventually accepted and integrated in the master branch.

you can share the details or you can open a PR to my Mainstream branch, then it will be added to this PR after merging.

As usual attached you can find all the files affected by my bug fixes and improvements.
I would like to avoid to submit a new PR by myself because it will probably require more time before it will be approved and eventually merged, while you are already the owner of some developments and fixes in the master branch, so it will take far less time. Furthermore you could also easily make further changes or improvements on that code

PR_changes.zip

[digant73]

@digant73
Good morning,
just as you might have expected, I am very curious about those fixes/ enhancements you posted about above, so is there any chance you would provide them?

thank you in advance
regards

zip file attached on my last reply to guruathwal

[discip]

@digant73
Thank you a ton! 😁
regards

[SeanLMH]

Hi, I've tried to update my TFT35 with this but it still doesn't seem to detect the mainboard firmware type (so I'm getting cold extrusions when slicing with prusaslicer), is there something special I need to do with my Marlin config? I'm using the TFT35 V3 E3 with an SKR Mini E3 V1.2