Fix kill => disable_all_heaters => print_job_timer.stop (#12146)

- Remove `print_job_timer.stop()` from `disable_all_heaters`
- Call `print_job_timer.stop()` for relevant `disable_all_heaters()`.
- Split up `kill()` for watchdog interrupt safety

Marlin/src/HAL/HAL_AVR/watchdog_AVR.cpp

@@ -63,9 +63,8 @@ void watchdog_init() {
   ISR(WDT_vect) {
     sei();  // With the interrupt driven serial we need to allow interrupts.
     SERIAL_ERROR_START();
-    SERIAL_ERRORLNPGM("Watchdog barked, please turn off the printer.");
+    SERIAL_ERRORLNPGM(MSG_WATCHDOG_FIRED);
-    kill(PSTR("ERR:Watchdog")); //kill blocks //up to 16 characters so it fits on a 16x2 display
+    minkill();  // interrupt-safe final kill and infinite loop
-    while (1); //wait for user or serial reset
   }
 #endif // WATCHDOG_RESET_MANUAL
 

Marlin/src/Marlin.cpp

@@ -351,7 +351,7 @@ void manage_inactivity(const bool ignore_stepper_queue/*=false*/) {
   if (max_inactive_time && ELAPSED(ms, gcode.previous_move_ms + max_inactive_time)) {
     SERIAL_ERROR_START();
     SERIAL_ECHOLNPAIR(MSG_KILL_INACTIVE_TIME, parser.command_ptr);
-    kill(PSTR(MSG_KILLED));
+    kill();
   }
 
   // Prevent steppers timing-out in the middle of M600
@@ -408,7 +408,7 @@ void manage_inactivity(const bool ignore_stepper_queue/*=false*/) {
     if (killCount >= KILL_DELAY) {
       SERIAL_ERROR_START();
       SERIAL_ERRORLNPGM(MSG_KILL_BUTTON);
-      kill(PSTR(MSG_KILLED));
+      kill();
     }
   #endif
 
@@ -609,7 +609,7 @@ void idle(
  * Kill all activity and lock the machine.
  * After this the machine will need to be reset.
  */
-void kill(PGM_P lcd_msg) {
+void kill(PGM_P const lcd_msg/*=NULL*/) {
   SERIAL_ERROR_START();
   SERIAL_ERRORLNPGM(MSG_ERR_KILLED);
 
@@ -617,23 +617,28 @@ void kill(PGM_P lcd_msg) {
   disable_all_steppers();
 
   #if ENABLED(EXTENSIBLE_UI)
-    UI::onPrinterKilled(lcd_msg);
+    UI::onPrinterKilled(lcd_msg ? lcd_msg : PSTR(MSG_KILLED));
   #elif ENABLED(ULTRA_LCD)
-    kill_screen(lcd_msg);
+    kill_screen(lcd_msg ? lcd_msg : PSTR(MSG_KILLED));
   #else
     UNUSED(lcd_msg);
   #endif
 
+  #ifdef ACTION_ON_KILL
+    SERIAL_ECHOLNPGM("//action:" ACTION_ON_KILL);
+  #endif
+
+  minkill();
+}
+
+void minkill() {
+
   _delay_ms(600); // Wait a short time (allows messages to get out before shutting down.
   cli(); // Stop interrupts
+  _delay_ms(250); // Wait to ensure all interrupts stopped
 
-  _delay_ms(250); //Wait to ensure all interrupts routines stopped
   thermalManager.disable_all_heaters(); // turn off heaters again
 
-  #ifdef ACTION_ON_KILL
-    SERIAL_ECHOLNPGM("//action:" ACTION_ON_KILL);
-  #endif
-
   #if HAS_POWER_SWITCH
     PSU_OFF();
   #endif
@@ -655,6 +660,7 @@ void kill(PGM_P lcd_msg) {
  */
 void stop() {
   thermalManager.disable_all_heaters(); // 'unpause' taken care of in here
+  print_job_timer.stop();
 
   #if ENABLED(PROBING_FANS_OFF)
     if (fans_paused) fans_pause(false); // put things back the way they were
@@ -979,9 +985,7 @@ void loop() {
         quickstop_stepper();
         print_job_timer.stop();
         thermalManager.disable_all_heaters();
-        #if FAN_COUNT > 0
+        zero_fan_speeds();
-          for (uint8_t i = 0; i < FAN_COUNT; i++) fan_speed[i] = 0;
-        #endif
         wait_for_heatup = false;
         #if ENABLED(POWER_LOSS_RECOVERY)
           card.removeJobRecoveryFile();

Marlin/src/Marlin.h

@@ -180,7 +180,8 @@ void disable_e_stepper(const uint8_t e);
 void disable_e_steppers();
 void disable_all_steppers();
 
-void kill(PGM_P);
+void kill(PGM_P const lcd_msg=NULL);
+void minkill();
 
 void quickstop_stepper();
 
@@ -218,6 +219,12 @@ extern millis_t max_inactive_time, stepper_inactive_time;
   #endif
 #endif
 
+inline void zero_fan_speeds() {
+  #if FAN_COUNT > 0
+    LOOP_L_N(i, FAN_COUNT) fan_speed[i] = 0;
+  #endif
+}
+
 #if ENABLED(USE_CONTROLLER_FAN)
   extern uint8_t controllerfan_speed;
 #endif

Marlin/src/core/language.h

@@ -131,6 +131,7 @@
 #define MSG_M115_REPORT                     "FIRMWARE_NAME:Marlin " DETAILED_BUILD_VERSION " SOURCE_CODE_URL:" SOURCE_CODE_URL " PROTOCOL_VERSION:" PROTOCOL_VERSION " MACHINE_TYPE:" MACHINE_NAME " EXTRUDER_COUNT:" STRINGIFY(EXTRUDERS) " UUID:" MACHINE_UUID
 #define MSG_COUNT_X                         " Count X:"
 #define MSG_COUNT_A                         " Count A:"
+#define MSG_WATCHDOG_FIRED                  "Watchdog timeout. Reset required."
 #define MSG_ERR_KILLED                      "Printer halted. kill() called!"
 #define MSG_ERR_STOPPED                     "Printer stopped due to errors. Fix the error and use M999 to restart. (Temperature is reset. Set it after restarting)"
 #define MSG_BUSY_PROCESSING                 "busy: processing"

Marlin/src/feature/I2CPositionEncoder.cpp

@@ -164,7 +164,7 @@ void I2CPositionEncoder::update() {
 
     #ifdef I2CPE_ERR_THRESH_ABORT
       if (ABS(error) > I2CPE_ERR_THRESH_ABORT * planner.settings.axis_steps_per_mm[encoderAxis]) {
-        //kill("Significant Error");
+        //kill(PSTR("Significant Error"));
         SERIAL_ECHOPGM("Axis error greater than set threshold, aborting!");
         SERIAL_ECHOLN(error);
         safe_delay(5000);

Marlin/src/gcode/control/M108_M112_M410.cpp

@@ -41,7 +41,7 @@ void GcodeSuite::M108() {
  * M112: Emergency Stop
  */
 void GcodeSuite::M112() {
-  kill(PSTR(MSG_KILLED));
+  kill();
 }
 
 /**

Marlin/src/gcode/control/M80_M81.cpp

@@ -23,6 +23,7 @@
 #include "../gcode.h"
 #include "../../module/temperature.h"
 #include "../../module/stepper.h"
+#include "../../module/printcounter.h" // for print_job_timer
 
 #include "../../inc/MarlinConfig.h"
 
@@ -95,10 +96,11 @@
  */
 void GcodeSuite::M81() {
   thermalManager.disable_all_heaters();
+  print_job_timer.stop();
   planner.finish_and_disable();
 
   #if FAN_COUNT > 0
-    for (uint8_t i = 0; i < FAN_COUNT; i++) fan_speed[i] = 0;
+    zero_fan_speeds();
     #if ENABLED(PROBING_FANS_OFF)
       fans_paused = false;
       ZERO(paused_fan_speed);

Marlin/src/gcode/queue.cpp

@@ -391,7 +391,7 @@ inline void get_serial_commands() {
               wait_for_user = false;
             #endif
           }
-          if (strcmp(command, "M112") == 0) kill(PSTR(MSG_KILLED));
+          if (strcmp(command, "M112") == 0) kill();
           if (strcmp(command, "M410") == 0) quickstop_stepper();
         #endif
 

Marlin/src/lcd/malyanlcd.cpp

@@ -254,9 +254,7 @@ void process_lcd_p_command(const char* command) {
         quickstop_stepper();
         print_job_timer.stop();
         thermalManager.disable_all_heaters();
-        #if FAN_COUNT > 0
+        zero_fan_speeds();
-          for (uint8_t i = 0; i < FAN_COUNT; i++) fan_speed[i] = 0;
-        #endif
         wait_for_heatup = false;
         write_to_lcd_P(PSTR("{SYS:STARTED}"));
       #endif

Marlin/src/lcd/ultralcd.cpp

@@ -1918,9 +1918,7 @@ void lcd_quick_feedback(const bool clear_buttons) {
   #endif // HAS_TEMP_HOTEND || HAS_HEATED_BED
 
   void lcd_cooldown() {
-    #if FAN_COUNT > 0
+    zero_fan_speeds();
-      for (uint8_t i = 0; i < FAN_COUNT; i++) fan_speed[i] = 0;
-    #endif
     thermalManager.disable_all_heaters();
     lcd_return_to_status();
   }

Marlin/src/module/endstops.cpp

@@ -36,6 +36,10 @@
   #include HAL_PATH(../HAL, endstop_interrupts.h)
 #endif
 
+#if ENABLED(ABORT_ON_ENDSTOP_HIT_FEATURE_ENABLED) && ENABLED(SDSUPPORT)
+  #include "../module/printcounter.h" // for print_job_timer
+#endif
+
 Endstops endstops;
 
 // public:
@@ -359,7 +363,8 @@ void Endstops::event_handler() {
         card.sdprinting = false;
         card.closefile();
         quickstop_stepper();
-        thermalManager.disable_all_heaters(); // switch off all heaters.
+        thermalManager.disable_all_heaters();
+        print_job_timer.stop();
       }
     #endif
   }

Marlin/src/module/temperature.cpp

@@ -304,7 +304,7 @@ uint8_t Temperature::soft_pwm_amount[HOTENDS];
 
     SERIAL_ECHOLNPGM(MSG_PID_AUTOTUNE_START);
 
-    disable_all_heaters(); // switch off all heaters.
+    disable_all_heaters();
 
     SHV(soft_pwm_amount, bias = d = (MAX_BED_POWER) >> 1, bias = d = (PID_MAX) >> 1);
 
@@ -779,7 +779,7 @@ void Temperature::manage_heater() {
   #endif
 
   #if ENABLED(EMERGENCY_PARSER)
-    if (emergency_parser.killed_by_M112) kill(PSTR(MSG_KILLED));
+    if (emergency_parser.killed_by_M112) kill();
   #endif
 
   if (!temp_meas_ready) return;
@@ -949,7 +949,7 @@ float Temperature::analog2temp(const int raw, const uint8_t e) {
       SERIAL_ERROR_START();
       SERIAL_ERROR((int)e);
       SERIAL_ERRORLNPGM(MSG_INVALID_EXTRUDER_NUM);
-      kill(PSTR(MSG_KILLED));
+      kill();
       return 0.0;
     }
 
@@ -1551,9 +1551,6 @@ void Temperature::disable_all_heaters() {
     pause(false);
   #endif
 
-  // If all heaters go down then for sure our print job has stopped
-  print_job_timer.stop();
-
   #define DISABLE_HEATER(NR) { \
     setTargetHotend(0, NR); \
     soft_pwm_amount[NR] = 0; \

Marlin/src/sd/cardreader.cpp

@@ -394,7 +394,7 @@ void CardReader::openFile(char * const path, const bool read, const bool subcall
         SERIAL_ERROR_START();
         SERIAL_ERRORPGM("trying to call sub-gcode files with too many levels. MAX level is:");
         SERIAL_ERRORLN((int)SD_PROCEDURE_DEPTH);
-        kill(PSTR(MSG_KILLED));
+        kill();
         return;
       }
 

Marlin/src/sd/usb_flashdrive/Sd2Card_FlashDrive.cpp

@@ -38,7 +38,7 @@ Sd2Card::state_t Sd2Card::state;
 
 // The USB library needs to be called periodically to detect USB thumbdrive
 // insertion and removals. Call this idle() function periodically to allow
-// the USB libary to monitor for such events. This function also takes care
+// the USB library to monitor for such events. This function also takes care
 // of initializing the USB library for the first time.
 
 void Sd2Card::idle() {