[BUG] Print pause not working with REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER disabled

[digant73]

Did you test the latest bugfix-2.0.x code?

Yes, and the problem still exists.

Bug Description

NOT WORKING SCENARIO:

Using a BTT TFT35 V3 in touch mode, pausing a print is not properly working with the following settings on Marlin:

• EMERGENCY_PARSER enabled
• HOST_PROMPT_SUPPORT enabled
• ADVANCED_PAUSE_FEATURE enabled
• PARK_HEAD_ON_PAUSE enabled
• REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER disabled

I sent a pause command M25 and I got:

1. the nozzle is moved to the park area as expected. The following dialog is provided by Marlin:
   //action:paused
   //action:prompt_end
   //action:prompt_begin Pause
   //action:prompt_button Dismiss
   //action:prompt_show

2. the nozzle is moved (very slowly) on top of the print

3. the print is resumed just for few seconds (1-2)

4. the print is paused on top of the print

Sending a resume command M24, I got:

1. the following dialog is provided by Marlin:
   //action:prompt_end
   //action:prompt_begin Nozzle Parked
   //action:prompt_button Continue
   //action:prompt_show

2. pressing on the "Continue" button, the following dialog is provided by Marlin:
   //action:prompt_end
   //action:prompt_begin Paused
   //action:prompt_button PurgeMore
   //action:prompt_button Continue
   //action:prompt_show

3. pressing on the "Continue" button, the following dialog is provided by Marlin:
   //action:resumed
   //action:prompt_end
   //action:prompt_begin Resuming
   //action:prompt_button Dismiss
   //action:prompt_show

4. the print is resumed

WORKING SCENARIO:

Pausing a print properly works with the following settings:

• EMERGENCY_PARSER enabled
• HOST_PROMPT_SUPPORT enabled
• ADVANCED_PAUSE_FEATURE enabled
• PARK_HEAD_ON_PAUSE enabled
• REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER enabled

So, the only difference compared with the not working scenario is only on setting REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER enabled instead of disabled

With the above settings, the print is properly paused and resumed according the following steps:

1. the nozzle is moved to the park area as expected. The following dialog is provided by Marlin:
   //action:paused
   //action:prompt_end
   //action:prompt_begin Pause
   //action:prompt_button Dismiss
   //action:prompt_show

2. the following dialog is provided by Marlin:
   //action:prompt_end
   //action:prompt_begin Nozzle Parked
   //action:prompt_button Continue
   //action:prompt_show

3. pressing on the "Continue" button, the following dialog is provided by Marlin:
   //action:prompt_end
   //action:prompt_begin Paused
   //action:prompt_button PurgeMore
   //action:prompt_button Continue
   //action:prompt_show

4. pressing on the "Continue" button, the following dialog is provided by Marlin:
   //action:resumed
   //action:prompt_end
   //action:prompt_begin Resuming
   //action:prompt_button Dismiss
   //action:prompt_show

5. the nozzle is moved on top of the print and the print is properly resumed

IMPORTANT NOTE:
In case of an M600 is sent, the auto print pause engaged by M600 works properly as in the "WORKING SCENARIO" .

Tested on:
Marlin bugfix 2021-05-04.

Hardware:
MKS GEN L v1.0
BTT TFT35 V3

Attached my configuration:

Marlin.zip

EDIT:
I found the route cause of the bug. With PARK_HEAD_ON_PAUSE enabled, M125 is called by M25.
On M125.cpp file, from line 83, the following code is present:

if (pause_print(retract, park_point, show_lcd, 0)) {
if (ENABLED(EXTENSIBLE_UI) || !sd_printing || show_lcd) {
wait_for_confirmation(false, 0);
resume_print(0, 0, -retract, 0);
}
}

The problem is due to condition "if (ENABLED(EXTENSIBLE_UI) || !sd_printing || show_lcd)" not matched (no LCD enabled, no extensible UI present, SD present in my printer). In that case the following code is not executed:

{
wait_for_confirmation(false, 0);
resume_print(0, 0, -retract, 0);
}

The above code (with different param values) is instead executed when function "pause_print" is invoked on M600.cpp file. That's the reason of the different behavior reported in my previous note above.
The different behavior is due to the recently merged PR #21671 that allowed M600 even in case NO LCD is enabled but EMERGENCY_PARSER and HOST_PROMPT_SUPPORT are enabled. That is my setup!

In order to fix the bug, it is enough to replace in M125.cpp the condition:

if (ENABLED(EXTENSIBLE_UI) || !sd_printing || show_lcd)

with (adding the condition in bold)

if (ENABLED(EXTENSIBLE_UI) || !sd_printing || show_lcd || ENABLED(M600_PURGE_MORE_RESUMABLE))

@thinkyhead, @thisiskeithb I saw you followed the PR #21671. Please could you verify the bug and fix it with the suggested solution? That fix is essential in order to merge a PR on BTT TFT fw (bigtreetech/BIGTREETECH-TouchScreenFirmware#1898).

Thanks

Bug Timeline

No response

Expected behavior

I expect the print pause is working as it is if REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER is enabled

Actual behavior

Print pause is not working as detailed in "NOT WORKING scenario" in the Bug Details page

Steps to Reproduce

No response

Version of Marlin Firmware

Bugfix version from 2021-05-04

Printer model

Artillery Sidewinder X1

Electronics

No response

Add-ons

No response

Your Slicer

Simplify3D

Host Software

No response

Additional information & file uploads

No response

[thisiskeithb]

Since you've provided a fix, it'd be faster if you submitted a Pull Request so it can be reviewed.

[guruathwal]

This does not look like a bug because the description specifically says it requires an LCD Display:

Marlin/Marlin/Configuration_adv.h

Lines 2303 to 2313 in 7378afc

	/**
	* Advanced Pause
	* Experimental feature for filament change support and for parking the nozzle when paused.
	* Adds the GCode M600 for initiating filament change.
	* If PARK_HEAD_ON_PAUSE enabled, adds the GCode M125 to pause printing and park the nozzle.
	*
	* Requires an LCD display.
	* Requires NOZZLE_PARK_FEATURE.
	* This feature is required for the default FILAMENT_RUNOUT_SCRIPT.
	*/
	//#define ADVANCED_PAUSE_FEATURE

[digant73]

@thisiskeithb I can submit a PR, but I was thinking that the owner of Marlin will speedup for a solution.
Just my thought about the current implementation of M125 (and the cause of the bug). If M125 (same form M600) requires a user interaction (so an LCD or prompt support) why not providing the check before calling M125?
So isn't it more correct, in M24_M25.cpp to replace:

#if ENABLED(PARK_HEAD_ON_PAUSE)

M125();

#else

with

#if ENABLED(PARK_HEAD_ON_PAUSE)

if (ENABLED(EXTENSIBLE_UI) || !sd_printing || show_lcd || ENABLED(M600_PURGE_MORE_RESUMABLE))
{
M125();
}

#endif

?

In that way, if user interaction can be provided (with LCD, with prompt, with both LCD and prompt), M125 will be called. Otherwise standard M25 will be used

[digant73]

@guruathwal the description needs to be updated. PR #21671 (from our well known BTT) was recently merged. It added the possibility to perform M125/M600 even in case only the prompt support is enabled in Marlin. And that feature makes absolutely sense because LCD enabled in Marlin takes alot of extra FLASH and RAM

[guruathwal]

@digant73 I tried to replicate your issue with the same settings as your NOT WORKING SCENARIO with the current bugfix branch of marlin and I found some strange results.
I tried to pause the print 8 times with M25 and resume with M24. During the test, with M25 the pause moved the hotend back from the parked position to the print area every second time.
The first time the hotend parked correctly, the second time the hotend moved back to the print area, and again the third time the hotend parked correctly and it kept happening like this in an alternate fashion.
The test file was a simple code file that moved the hotend in a repeated rectangular pattern. let's say the 4 the hotend moves between 4 points A, B, C & D.
During the unexpected behaviour, when I send M25 to pause, assuming that the last gcode in buffer was to move the hotend from Point A to point B, the hotend will reach point B and then move to the park location.
After reaching the parked location the hotend will slowly move to point C and stay there.
When I sent M24 to resume the print, the hotend moved back to point B and then moved diagonally to point D skipping point C entirely.
With these findings, I doubt it is related to the missing ENABLED(M600_PURGE_MORE_RESUMABLE) because in the code block in M125.cpp:

Marlin/Marlin/src/gcode/feature/pause/M125.cpp

Lines 83 to 87 in 76ec704

	if (pause_print(retract, park_point, show_lcd, 0)) {
	if (ENABLED(EXTENSIBLE_UI) || !sd_printing || show_lcd) {
	wait_for_confirmation(false, 0);
	resume_print(0, 0, -retract, 0);
	}

The pause is executed by pause_print(retract, park_point, show_lcd, 0).
The line: wait_for_confirmation(false, 0) is loop for waiting and catching the user input on marlin LCD and resume_print(0, 0, -retract, 0); is the same executed by M24.
So even if these two lines were enabled or not, they should have no effect because the resume is being executed by M24 through GcodeSuite::M24().
The issue seems to be related to the print/planner buffer, although I might be wrong. @thinkyhead can you spare some time to look into this issue?

[digant73]

yes, that was the same I saw and looking at the code there is something else that caused that movement. But they seems to me like something related to a missing handling after the print pause function that is fixed in case "wait_for_confirmation" or "resume_print" is invoked. The fix I applied was mainly to allow the same logic currently applied for M600 for all possible scenarios. The fix I just pushed seems solving both the problems (the starnge movement and my main problem to allow the user to resume the print with a dialog interaction instead of using M24. Sure a double checks on furher possible bugs (not related to my main problem) could still be present.

[guruathwal]

I doubt that fix is a good approach because wait_for_confirmation(false, 0); is specifically written for user input via LCD display.
The hotend ultimately stops and waits, so it is somewhere in the pause function.

[digant73]

"wait_for_confirmation" and "resume_print" are the same used also for M600 that is properly working even in the NOT WORKING scenario. They support both LCD and prompt dialogs. Sure a review in some functions to discover other possible bugs is welcome

[digant73]

Just made further testing and checks in the code. the printPause function works properly in conjunction with resume_print functions. In particular the resume_print function avoids the starnge movement. So the PR #21813 fixes two problems (the target bug and the strange nozzle movement) in case prompt is supported.
Although the PR is an improvement to the current broken logic and should be not blocked for the merge, I will continue to check the printPause and resume_print and fix the bug when found

[thinkyhead]

M125 P is presumed with ExtUI so maybe it can also be presumed with Host Prompts. However, there is no guarantee of a connected host in the same way as we are guaranteed a connected LCD when LCD-related options are enabled. So, the trick is to see how the patched code works in the case of M125 sent from a host that has no idea about host prompts.

[digant73]

Yes. The BTT TFT fw supports prompt and sends M25 P1 and from TFT fw I found the bug (the bug was not present in case the pause function was invoked during an M600).
For the M25, I will also check on TFT fw

[digant73]

@thinkyhead I think there is no problem with M125 because M125 is available only if PARK_HEAD_ON_PAUSE is enabled but that means also ADVANCED_PAUSE_FEATURE enabled and M600 always needs an LCD or Host Prompts (with the recently merged PR from BTT). For that reason I extended the same condition (LCD or Host Prompts) available for M600 also to M125.

[thinkyhead]

Well, if chains of options guarantee it, then I guess we are safe! Once we get through bug reports on the new release we're going to write up all the configuration options for the website, and we'll make a point to spell out all those kinds of dependencies.

[guruathwal]

So, the trick is to see how the patched code works in the case of M125 sent from a host that has no idea about host prompts.

@thinkyhead
Printing from SD card via Repetier host with no display defined in Marlin and enabling PARK_HEAD_ON_PAUSE along with ADVANCED_PAUSE_FEATURE, the pause works fine but resume never works. Manually sending M876 S1 does not work, this may be because of the continuous echo:busy: paused for user message. Ultimately the hotend timeout is triggered.

Printing from SD card via PrintRun/PronterFace with same Marlin configuration, the pause works fine and after pressing the resume button M876 S1 has to be sent manually to resume printing. M876 S1 again needs to be sent If the heater timeout because there is no notification other than the text received in the terminal.

[digant73]

@thinkyhead or just to be fully compliant with M25/M125 and M25/M125 P1 reported in the description of M125 function, in M125.cpp we could use the following code (in bold the new code).

const bool support_dialog = (ENABLED(HAS_LCD_MENU) || ENABLED(EXTENSIBLE_UI) || BOTH(EMERGENCY_PARSER, HOST_PROMPT_SUPPORT));
const bool show_dialog = support_dialog ? parser.boolval('P') : false;

// If possible, show an LCD prompt with the 'P' flag
const bool show_lcd = TERN0(HAS_LCD_MENU, parser.boolval('P'));

TERN_(POWER_LOSS_RECOVERY, if (recovery.enabled) recovery.save(true));

if (pause_print(retract, park_point, show_lcd, 0)) {
// if (ENABLED(EXTENSIBLE_UI) || BOTH(EMERGENCY_PARSER, HOST_PROMPT_SUPPORT) || !sd_printing || show_lcd) {
if (show_dialog || (!sd_printing && support_dialog)) {
wait_for_confirmation(false, 0);
resume_print(0, 0, -retract, 0);
}
}

In that way:

• if M25 P1/M125 P1 is invoked and a dialog with user can be provided (with LCD or ExtUI or prompt), then the dialog is requested (no M24 is needed to be sent)
• if M25 P1/M125 P1 is invoked but no dialog with user can be provided (no LCD, no ExtUI, no prompt), then no dialog is requested and M24 is needed to be sent to resume the print
• if M25/M125 is invoked, then no dialog is requested and M24 is needed to be sent to resume the print
• if not actively printing and a dialog with user can be provided, then the dialog is requested (no M24 is needed to be sent). If dialog is not supported, then M24 is needed to be sent.

The description of M125 should be slightly changed reporting option P1 needs an LCD or ExtUI or prompt support

Just tested the above code also with M25 and M25 P1 form BTT TFT fw

[github-actions]

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.