Print Pause disrupts host communication

[minosg]

Bug Description

Pausing the printer, when printing from SD, or Host, will eventually end up disabling serial out commands:

• SERIAL_PRINT()
• SERIAL_ECHOPGM()
• SERIAL_ECHO()
• serialprintPGM()
• Everything else defined in Core/Serial.h will stop working

This will in turn affect host action commands, since when the printer is paused, resume and kill operations performed by the LCD display will not be broadcast to host.

The issue manifests itself after the auto-bed leveling routine is complete, and the comms are working at this early stage. Also if you pause and unpause the job fast enough it will work. But if the printer is left paused until the block queue if complete, the SERIAL_XXX commands will stop to function.

My Configurations

configs.zip

Steps to Reproduce

1. Compile firmware with host action commands
2. Start a print using octoprint
3. Pause the print using the display menu
4. Wait for printer to stop moving
5. Attempt to resume or kill the print using the lcd menu.

Expected behavior:
I except the printer to receive the command and act on it

Actual behavior:
Nothing is clocked in the uart on the receiver side. Command never arrives, so host action commands are breaking.

Additional Information

Better way to test

1. Add those random print statements on MarlinUI::resume_print()
2. Compile and connect to Serial to the printer
3. Start a print from SD (faster/easier) or Octoprint
4. Test that the statements work during pre-heating and auto-bed leveling
   by pausing resuming the printer and checking the serial output.
5. Once the fisrt layer starts, pause the print. Wait for printer to stop moving
6. Press resume. No text is received by host.

  void MarlinUI::resume_print() {
    SERIAL_ECHOPGM("Test1\n");
    SERIAL_ECHO("Test2\n");
    SERIAL_FLUSH();
    serialprintPGM("Test3\n");
    reset_status();
    #if ENABLED(PARK_HEAD_ON_PAUSE)
      wait_for_heatup = wait_for_user = false;
    #endif
    if (IS_SD_PAUSED()) queue.inject_P(M24_STR);
    #ifdef ACTION_ON_RESUME
      host_action_resume();
    #endif
    print_job_timer.start(); // Also called by M24
    serialprintPGM("Test4\n");
  }

Because host actions rely on the channel being open, even though the data are placed in the buffer, they are somehow lost.

host_actions.cpp

void host_action(PGM_P const pstr, const bool eol) {
  SERIAL_ECHOPGM("//action:");
  serialprintPGM(pstr);
  if (eol) SERIAL_EOL();
}

Actions

1. Determine if that platform specific or weather it affect multiple platforms. It affects BBT SKR Turbo ( [BUG] "FILAMENT_RUNOUT_SCRIPT M600" does not send host action commands after filament runout #18209 ) as well as BTT SKR mini (STM32F1) it less likely to be a platform issue .
2. Determine if it is a host issue. Different operating systems have been tested, and line peaked for raw bytes using a serial analyses. No data is coming out of the USB cable.
3. Determine if it a recoverable state. It is a recoverable state, if extra logic is added to the idle() handler, to start printing stuff to serial a second after the block queue has started, it will properly transmit the data. That will not help when using octoprint though, since the command to start sending gcode will never be received, if the user uses the LCD to resume a print/perform a filament swap
4. Determine if it can be fixed by starting the print_job_timer.start() and waiting an x amount of time before printing. It is not fixed by this action alone

Theory

When a print job is paused I see two things happening directly. The print job timer is stopped and if printing from SD an pause flag is asserted. This this issue is triggered in both SD and HOST prints, the SD flag should be not directly responsible.

When printing from SD the pause logic is injecting an m25 command to the queue, and by the time the M25 is invoked the comms are down.

Since the effects happens a few milliseconds after you pause, and lines before that will be clocked out of the Serial channel, I suspect the core culprit is the queue class, I am not certain where and how but it would be possible that serial output is misdirected.

Alternatively there could be a sanity check which is intentionally stopping serial commands during print pause, or host keepalive was meant to trigger and never did?

[minosg]

Apparently this has to do with the serial traffic being routed to the wrong interface. I am not certain as to why

Workaround/ Temporary Fix

You can get around the issue by overriding the PORT_REDIRECT for MarlinUI::resume_print() and MarlinUI::abort_print()

It would be advised to call PORT_RESTORE in the end to undo this change, even though I have not found any indication of affecting anything

  void MarlinUI::resume_print() {
    #if ENABLED(HOST_ACTION_COMMANDS)
      PORT_REDIRECT(SERIAL_BOTH);
    #endif
    reset_status();
    TERN_(PARK_HEAD_ON_PAUSE, wait_for_heatup = wait_for_user = false);
    if (IS_SD_PAUSED()) queue.inject_P(M24_STR);
    #ifdef ACTION_ON_RESUME
      host_action_resume();
    #endif
    print_job_timer.start(); // Also called by M24
    #if ENABLED(HOST_ACTION_COMMANDS)
      PORT_RESTORE();
    #endif
  }

  void MarlinUI::abort_print() {
    #if ENABLED(HOST_ACTION_COMMANDS)
      PORT_REDIRECT(SERIAL_BOTH);
    #endif
    #if ENABLED(SDSUPPORT)
      wait_for_heatup = wait_for_user = false;
      card.flag.abort_sd_printing = true;
    #endif
    #ifdef ACTION_ON_CANCEL
      host_action_cancel();
    #endif
    TERN_(HOST_PROMPT_SUPPORT, host_prompt_open(PROMPT_INFO, PSTR("UI Aborted"), DISMISS_STR));
    print_job_timer.stop();
    set_status_P(GET_TEXT(MSG_PRINT_ABORTED));
    TERN_(HAS_LCD_MENU, return_to_status());

    #if ENABLED(HOST_ACTION_COMMANDS)
      PORT_RESTORE();
    #endif
  }

[thinkyhead]

I think you actually have the correct fix, or at least one possible one. Marlin tries to be conservative and only send serial replies back to the original source. But with these external displays also being serial hosts, it gets a little more complicated. Presumably the Malyan LCD will ignore host strings and will only care about well-formed strings with the necessary colons and braces, but it's hard to predict what newer screens might do.

[thinkyhead]

I've patched bugfix-2.0.x so that all host actions are done as a broadcast to both serial ports. It should make host actions work properly, but please test to confirm.

[minosg]

@thinkyhead I saw the patch and on first thoughts it should work since originally my workaround was tested in a similar fashion.

I will test it over the weekend when I'm back home and confirm ASAP.

[minosg]

@thinkyhead

I have just re-based and verified that you patch works in STM32F1 boards. since its a core and not a hal change it should work across the board.

[sjasonsmith]

@minosg based on your testing I'll close this issue. Please let me know if there was still a reason to keep it open.