Fix Host Keepalive serial output with multi-serial

[rhapsodyv]

Description

This is a continuation of the stress tests I'm doing on multi serial, to fix corner cases.

I found an issue that happens in a very specific combination of events, but that can happen very often on a real printing operation: multi serial + idle stacking (slow commands that call idle until finished) + keep alive code.

When marlin is executing some slow commands, it may stay in a loop waiting for it to complete. Inside that loop, marlin may call idle periodically. So, the idle will check for new serial commands (in the other serial) to enqueue. When it receive the command, it will reply "busy" (on keep alive function), but it send to the wrong serial port.

Call stack:

idle
  process_next_command
    Gxxx
       while(something)
          idle
            keepalive (sent to the current serial)
            other serial tries to send data, thinks it succeeds, but marlin simply doesn't reply (until the actual commands)
            data loss and wrong behavior on the first serial (it will cause resend on serial 1)

I could simulate resend commands on octoprint this way. And I fixed making the keep alive reply "busy" for all serial ports...

That in fact it seems correct, because marlin will only handle one command at time, so the keep alive should warns all serial ports that marlin is busy, to avoid timeout or false-resend commands everywhere.

Benefits

More robust multi serial.
May fix #21010 and #21244

Related Issues

#21249

[thinkyhead]

It looks like multiSerial.portMask is being left uninitialized (zero) so the fallback for SERIAL_REDIRECT is no ports. The fallback should probably be 0xFF, or SERIAL_ALL, and this might actually be the correct fix.

[rhapsodyv]

It looks like multiSerial.portMask is being left uninitialized (zero) so the fallback for SERIAL_REDIRECT is no ports. The fallback should probably be 0xFF, or SERIAL_ALL, and this might actually be the correct fix.

yes, can be. But remember that the second idle is called inside another running command.... so, it may set the serial port to the current command... and the other command just hang (it's what happens).

[rhapsodyv]

Test:

Octo "printing" in one serial 3 (only g1 x2 x2)

Then, in another serial (serial 1), I send a long move, to generate a wait on planner, to it call idle and get a keep alive:

This is what happens on octo:

Send: N4848 G1 X2.372 Y2.8189999*30
Recv: ok
Send: N4849 G1 X2.372 Y2.8189999*31
Recv:  T:-15.00 /0.00 @:0
Recv: echo:busy: processing. <<<<-receive the busy
Printer seems to support the busy protocol, will adjust timeouts and set busy interval accordingly
Recv:  T:-15.00 /0.00 @:0
Recv: ok
Send: N4850 M113 S2*88
Recv:  T:-15.00 /0.00 @:0
Recv:  T:-15.00 /0.00 @:0
Recv:  T:-15.00 /0.00 @:0
Recv:  T:-15.00 /0.00 @:0
Recv:  T:-15.00 /0.00 @:0
Recv:  T:-15.00 /0.00 @:0
Recv:  T:-15.00 /0.00 @:0
Recv:  T:-15.00 /0.00 @:0
Recv:  T:-15.00 /0.00 @:0
Recv:  T:-15.00 /0.00 @:0
Recv:  T:-14.92 /0.00 @:0
Recv:  T:-15.00 /0.00 @:0
Recv:  T:-15.00 /0.00 @:0
Recv:  T:-15.00 /0.00 @:0
Recv:  T:-15.00 /0.00 @:0
Recv:  T:-15.00 /0.00 @:0

Communication timeout while printing, trying to trigger response from printer. Configure long running commands or increase communication timeout if that happens regularly on specific commands or long moves.

Send: N4851 M105*31
Recv: Error:checksum mismatch, Last Line: 4849
Recv: Resend: 4850
Send: N4850 M113 S2*88
Recv: ok
Send: N4851 M105*31
Recv: ok T:-15.00 /0.00 @:0

Send: N4852 G1 X2.372 Y2.8189999*21

After adding the like that send busy for all serial ports, octo print will get hang or get timeout

[rhapsodyv]

This behaviour happened always in my tests: every time I generate a wait having commands for another serial, one serial loses a command and get timeout. With busy broadcast, I got no more resend erros on octo, even if I send commands in loop to the two serials, generating lots of busy, etc.

[thinkyhead]

But, PORT_REDIRECT carries its own "stack" with it to ensure that everything is using the port it expects. It creates an object that restores the previous serial mask when it goes out of scope. If it isn't working now, maybe it's not restoring the previous value properly.

[rhapsodyv]

But, SERIAL_REDIRECT carries its own "stack" with it to ensure that everything is using the port it expects. It creates an object that restores the previous serial mask when it goes out of scope. If it isn't working now, maybe it's not restoring the previous value properly.

Yes, but its value will be the last value set, right? That is the value of the current command being executed.

You can easy test it:
in the serial 1, send a M0. This serial will be receiving paused by the user every "keep alive seconds"

then, In the serial 2, send a M114... Marlin will not respond anything to this serial, never..... even if, in fact, marlin is currently busy... So, this serial will timeout, if the command is queued... so, after the serial 1 release, marlin will execute the M114, and the host may send it again!!

That is the reason I sent the PR, because makes sense, for me, that keep alive should broadcast (as all host commands).

But yes, maybe we have more hidden issues hehehe...

[thinkyhead]

The current command does redirect its output, and presumably it wants to also redirect the output of anything within idle that is intended as a message about the current state. Things called by idle() will be responsible for redirecting to SERIAL_ALL if they need to. When the running command is complete it should be restoring the serial port mask back to the default (i.e., SERIAL_ALL).

[thinkyhead]

…so you are correct to redirect the output of keepalive.

[thinkyhead]

And now, to fix this exp32 compile before I go ahead and merge….

[rhapsodyv]

I found ... the lib arduinoWebSockets was updated to use the lasts code from espressif32... but we are using [email protected].... Or we try to use old version of arduinoWebSockets or new version of espressif32...

we can point arduinoWebSockets to one version old and it will work...
On features, point arduinoWebSockets to 2.3.4 (current is 2.3.5, committed yesterday)

arduinoWebSockets=https://github.com/Links2004/arduinoWebSockets/archive/2.3.4.zip

[adolson]

bugfix-2.0.x fails to compile in Arduino IDE after this merge. Was working yesterday. How do I fix this?

In file included from sketch/src/HAL/ESP32/FlushableHardwareSerial.cpp:23:0:
sketch/src/HAL/ESP32/FlushableHardwareSerial.h: In constructor ‘FlushableHardwareSerial::FlushableHardwareSerial(int)’:
sketch/src/HAL/ESP32/FlushableHardwareSerial.h:31:64: error: no matching function for call to ‘HardwareSerial::HardwareSerial(int&)’
   FlushableHardwareSerial(int uart_nr) : HardwareSerial(uart_nr) {}
                                                                ^
In file included from sketch/src/HAL/ESP32/FlushableHardwareSerial.h:24:0,
                 from sketch/src/HAL/ESP32/FlushableHardwareSerial.cpp:23:
/usr/share/arduino/hardware/arduino/avr/cores/arduino/HardwareSerial.h:117:12: note: candidate: HardwareSerial::HardwareSerial(volatile uint8_t*, volatile uint8_t*, volatile uint8_t*, volatile uint8_t*, volatile uint8_t*, volatile uint8_t*)
     inline HardwareSerial(
            ^
/usr/share/arduino/hardware/arduino/avr/cores/arduino/HardwareSerial.h:117:12: note:   candidate expects 6 arguments, 1 provided
/usr/share/arduino/hardware/arduino/avr/cores/arduino/HardwareSerial.h:93:7: note: candidate: constexpr HardwareSerial::HardwareSerial(const HardwareSerial&)
 class HardwareSerial : public Stream
       ^
/usr/share/arduino/hardware/arduino/avr/cores/arduino/HardwareSerial.h:93:7: note:   no known conversion for argument 1 from ‘int’ to ‘const HardwareSerial&’
/usr/share/arduino/hardware/arduino/avr/cores/arduino/HardwareSerial.h:93:7: note: candidate: constexpr HardwareSerial::HardwareSerial(HardwareSerial&&)
/usr/share/arduino/hardware/arduino/avr/cores/arduino/HardwareSerial.h:93:7: note:   no known conversion for argument 1 from ‘int’ to ‘HardwareSerial&&’
exit status 1
Error compiling for board Arduino Mega or Mega 2560.

[rhapsodyv]

@adolson take a look if it solves: https://github.com/rhapsodyv/Marlin/tree/21283-followup

[adolson]

Yes, that compiles successfully.