Fix: temperature auto-report timeout check, Fix invalid settings parameters

[guruathwal]

• Fix for temperature auto-report disabling due to Mainboard reset or through any other serial controller ([BUG] Bad hotend and bed temperature update on status screen. #1434). Now it checks for a timeout after last reported temperature status and resends M155 command.
• Update old settings signature causing invalid parameter reading due to changes in parameters via PR Print finished #1359 (related detail in [BUG] Screen freezes in Marlin mode while starting to draw status screen. #1433 (comment))
• Update config.ini
• Fix leveling feed-rate for probe height/offset control
• Clean code for marlin mode
• add missing xy_offset_probing parsing in config.c

Fixes #1434

[kisslorand]

Great work! It also removes the "No printer attached!" reminder after the motherboard is disconnected and reconnected.
Yummy!

[kisslorand]

heat_update_waiting = storeCmd("M155 ");
if(heat_update_waiting) updateNextHeatCheckTime();

The order should be reversed like above, otherwise "if(heat_update_waiting) updateNextHeatCheckTime()" is never executed.

[guruathwal]

🤦‍♂️it was supposed to be this way but somehow the lines got interchanged accidentally.

[kisslorand]

With this PR if you send M155 S0 from TFT, it will be sent periodically forever, without a stop. If you try to set M155 Sx from outside, directly to the mainboard (OctoPrint, Pronterface) the TFT will send again M155 S0, so M155 cannot be enabled again from outside, only from the TFT.
So, if M155 was switched off by a deliberate action, automatically receiving temp informations should not be checked again.

I suggest:
if ((OS_GetTimeMs() > AUTOREPORT_TIMEOUT) && (!heat_update_waiting) && (heat_update_seconds > 0))

If M155 is switched off, maybe "infoMachineSettings.autoReportTemp" should also be set to "0" and switched back to "1" as soon as M155 is enabled again.

[AntoszHUN]

Another fix? Super! Maybe you can help me why you don't update the layer height during printing?

It is clear from the picture that it is not 0.8mm. True?

[kisslorand]

This was fixed 9 days ago when PR #1394 was merged into master.
It was caused in PrintingMenu.c by
if(curLayer != (infoFile.source == BOARD_SD) ? coordinateGetAxisActual(Z_AXIS) : coordinateGetAxisTarget(Z_AXIS))
which has been corrected to
if (curLayer != ((infoFile.source == BOARD_SD) ? coordinateGetAxisActual(Z_AXIS) : coordinateGetAxisTarget(Z_AXIS))).

[AntoszHUN]

This was fixed 9 days ago when PR #1394 was merged into master.
It was caused in PrintingMenu.c by
if(curLayer != (infoFile.source == BOARD_SD) ? coordinateGetAxisActual(Z_AXIS) : coordinateGetAxisTarget(Z_AXIS))
which has been corrected to
if (curLayer != ((infoFile.source == BOARD_SD) ? coordinateGetAxisActual(Z_AXIS) : coordinateGetAxisTarget(Z_AXIS))).

I'll see then. Thanks! I haven’t had time to upgrade lately.

[kisslorand]

The fix for config.ini and inital baudrate is:

In config.ini:
#### Baudrate
#Optiuns: [2400, 9600, 19200, 38400, 57600, 115200, 250000, 500000, 1000000]
#
baudrate:115200

and in config.c:
case C_INDEX_UART_BAUDRATE:
__SET_VALID_INT_VALUE(infoSettings.baudrate, item_baudrate[0], item_baudrate[ITEM_BAUDRATE_NUM - 1]);
__break;

Leave Configuration.h as it was.
*Optiuns: [2400, 9600, 19200, 38400, 57600, 115200, 250000, 500000, 1000000]
*/
#define BAUDRATE 115200

The flash is expected to store the actual baudrate, not its order in a list.
This wasn't caused by "Update old settings signature causing invalid parameter reading due to changes in parameters via PR #1359".

Fixed in PR #1441

[oldman4U]

Hi Gurmeet.

Hope you are fine.

So this PR is going to change the way the "kind" of menu used is "named", right?

From:
Unified Menu / Classic Menu
Select a UI menu flavour
Options: [Unified Menu: 1, Classic Menu: 0]
unified_menu:1
to:
Enable Status Screen
Status screen or home screen displays the current temperature, fan and speeds.
If this disabled, the main menu will become the default home screen.
Options: [Enable: 1, Disable: 0]
status_screen:1

Is there also any kind of functionality or look which changes or is it really only how it is named in config.ini?

[guruathwal]

@kisslorand The parameters were read wrong after #1359 af86e84a7 because the setting signatures in flashstore.c was not updated after removal of print_summary which caused wrong reading of subsequent parameters listed after print_summary.

Yes, the problem for the baud-rate issue was in config.c. The logic for reading and validating the baud-rate settings should not be as complex as it is. The baud-rate is checked against the predefined list, but in the config file, The option asks for a value instead of the index. The firmware compares the value with all the available options to finally store the value only if it exists in the predefined list. What is the point of asking to enter the value when it is the index which suffices? This also gives it ambiguity that any value can be provided for the baud-rate. The same issue can occur with the original code if the baud-rate is set to any other value not provided in the list. The logic was changed by me a long time ago when I added the config file feature but it was changed again sometime after that.

@oldman4U yes, the icons for the classic menu and the menu layout were removed so there is no classic menu any more. The only thing that option now does is switch the status screen on and off.

[kisslorand]

@kisslorand The parameters were read wrong after #1359 af86e84a7 because the setting signatures in flashstore.c was not updated after removal of print_summary which caused wrong reading of subsequent parameters listed after print_summary.

If there's a config.ini, the reading and storing of parameters from config.ini restores the order of the subsequent parameters.
flashStore.c -> storePara()

All I said that PR #1359 is not the cause of baudrate parameter being wrong after config.ini is loaded.

Yes, you are right, any value can be provided as baudrate, which might be fine if someone uses a non-standard baudrate in its Marlin. Same goes for selecting the index of the baudrate, a user can provide also the wrong index. Using indexes makes the TFT to work only at a few predefined baudrates, which is totally fine for general usage.
I do not see anything wrong storing the real baudrate nor storing the index from a predefined list. I just found the bug, I didn't created it. :)
An index was stored and that stored index was used directly as baudrate.

In case of storing only the index the fix would be:

• in Configuration.h:

*Options: [2400: 0, 9600: 1, 19200: 2, 38400: 3, 57600: 4, 115200: 5, 250000: 6, 500000: 7, 1000000: 8]
*/
#define BAUDRATE 5

and use "item_baudrate[infoSettings.baudrate]" instead of "infoSettings.baudrate" later in the whole code., where the real baudrate info is needed.

[oldman4U]

kisslorand used #1359 to silently remove a feature mehmetsutas and I have been introducing after spending days and hours of developing and testing, because he does not like it. By doing so, he messed up config.ini.

@guruathwal In case there is no technical reason for removing this feature, would you please be so kind and reverse what kisslorand did and reinstall the ability to choose, if a print finished information should be shown at the end of a print or not.

Thank you

[kisslorand]

@guruathwal
Check PR ##1441, it has been reworked to adapt your ideas about using only indexes. Thank for insisting, it seems better this way, even the compiled flash is smaller now.