M410 out of the UART IQR

[rhapsodyv]

Description

The Emergency Parser run inside the UART IRQ. So, the others IRQ (like temperature timer) will only run when EP returns.

For this reason, EP cannot run any command that depends on other IRQ.

As quickstop_stepper(), that depends on temperature timer, to finish and return.

This PR makes M410 exec in a async way (moved to idle), as other EP commands. This will allow the quickstop_stepper() run, while keep all other IRQ enabled and running too.

Benefits

Fix #19490

Configurations

Any board with EP enabled.

Related Issues

#19490

[schlaegerz]

With this fix, if you send a M410 while homing it only cancels the current homing move. I'm not sure if this is an existing problem or specific to this PR due to this issue #19490

[rhapsodyv]

With this fix, if you send a M410 while homing it only cancels the current homing move. I'm not sure if this is an existing problem or specific to this PR due to this issue #19490

The bug is: M410 crash board. Reason: it was executed inside a IRQ. Fix: execute out of a IRQ. With my fix, M410 will run properly, always.

Now, if M410 don't work with homing, it is not related with my fix, neither with the original issue report. It's M410 that never worked with homing (I didn't tested homing+M410).

[rhapsodyv]

I moved M410 to run in the same place as M112. So, if M112 run, M410 will run.

[jprogin]

I observed the same M410 crash issue, but I have solved it in another way (before to find this pull request).

I have suspected a stack overflow due to the call of the idle command. Thus I have protected the get_serial_commands in this way:

void GCodeQueue::get_serial_commands() {
    static uint8_t already_running = 0;
    if(already_running) return;
    already_running = 1;
    // ...
    already_running = 0;
}

And it seems to solve it.

[sjasonsmith]

I have protected the get_serial_commands in this way

Some menu screens do something similar, if they call idle() which may in-turn try to process the same screen again.

[rhapsodyv]

Emergency Parser don’t use get_serial_command. You may have found a bug in M410 when used without emergency parser.

The problem with M410 and Emergency Parser is that it was running inside the serial IRQ handler, blocking others IRQ to run. And the M410 will only return after 1000 temperature timer run. But as irq are blocked, the temperature timer never run, and it stay forever in loop.

Seems that M410 don’t work while homing to. I didn’t tested my self.

Anyway, this PR make M410 work exactly as M112.

[rhapsodyv]

Here is a log of the new code, with EP enabled. I added a lot to show how many times the M410 was called (to check if is there any stacked call).
For me, seems 100% right:

echo:start
PowerUp
Marlin bugfix-2.0.x

echo: Last Updated: 2020-10-15 | Author: (none, default config)
echo:Compiled: Oct 16 2020
echo: Free Memory: 8119  PlannerBufferBytes: 1280
echo:Hardcoded Default Settings Loaded
echo:  G21    ; Units in mm (mm)

echo:; Filament settings: Disabled
echo:  M200 S0 D1.75
echo:; Steps per unit:
echo: M92 X80.00 Y80.00 Z4000.00 E500.00
echo:; Maximum feedrates (units/s):
echo:  M203 X300.00 Y300.00 Z5.00 E25.00
echo:; Maximum Acceleration (units/s2):
echo:  M201 X3000.00 Y3000.00 Z100.00 E10000.00
echo:; Acceleration (units/s2): P<print_accel> R<retract_accel> T<travel_accel>
echo:  M204 P3000.00 R3000.00 T3000.00
echo:; Advanced: B<min_segment_time_us> S<min_feedrate> T<min_travel_feedrate> J<junc_dev>
echo:  M205 B20000.00 S0.00 T0.00 J0.01
echo:; Home offset:
echo:  M206 X0.00 Y0.00 Z0.00
echo:; PID settings:
echo:  M301 P22.20 I1.08 D114.00
echo:SD card ok
echo:SD card ok
M119
Reporting endstop status
x_min: TRIGGERED
y_min: TRIGGERED
z_min: TRIGGERED
ok
M410
quickstop_stepper start
quickstop_stepper end
ok

[rhapsodyv]

Now M410 without EP. It is not stacking calls too. It only gives a strange echo:Unknown command: "". But this is not related to my changes, as without EP, it runs normally in the command queue.

echo:  M301 P22.20 I1.08 D114.00
echo:SD card ok
echo:SD card ok
M119
Reporting endstop status
x_min: TRIGGERED
y_min: TRIGGERED
z_min: TRIGGERED
ok
M410
quickstop_stepper start
quickstop_stepper end
echo:Unknown command: ""
ok

[rhapsodyv]

In resume: I could not simulate the stacked calls and everything is working for me. I didn't see any new behaviour.

[rhapsodyv]

I did some debugging, and the echo:Unknown command: "" is just because my terminal is sending \r+\n, so a \n is processed as a second command... It's not a M410 issue.

[borsegbr]

in endstop.cpp you can add at line 319 to solve homming failed problem

 #if ENABLED(EMERGENCY_PARSER)
      if (!emergency_parser.quickstop_by_M410)
    #endif

[rhapsodyv]

endstop.cpp

No, the validate_homing_move method will only work if I call kill, to not return anymore from it. You can see that it just return void. Returning from it means success. But if you want kill, you can just send M112.

M410+Homing is a lot tricker. Sure it can be solve with a few ifs in the middle of the code. But the problem seems a bit deeper than that, as M410 just stop one move, and the control will get back to the current task, generating a undefined behaviour, possibly, in more cases.

And, it's subject for another issue, specific for that, as we may need a bigger rework and testing, very different from what this PR is addressing.