From fb025bba058a03461e01fdef1444834d4d7dade1 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Tue, 7 Dec 2021 11:06:08 +0100 Subject: [PATCH 01/10] Introduce severity levels for alert messages Use the internal lcd_status_message_level for multiple severity levels of alert messages. This is needed to distinguish between non-critical alerts (such as FAN ERROR) from critical ones (any heater issue). During a failure scenario, a critical error MUST NOT be overridden by a lower-level one. As such LCD_STATUS_CRITICAL is currently used for all heater-related errors that result in a safety full-stop. --- Firmware/ultralcd.cpp | 20 ++++++++++++-------- Firmware/ultralcd.h | 13 ++++++++++--- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index dfaa04ca3..fcce9b199 100755 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -8813,18 +8813,22 @@ void lcd_updatestatus(const char *message){ lcd_draw_update = 1; } -void lcd_setalertstatuspgm(const char* message) +void lcd_setalertstatuspgm(const char* message, uint8_t severity) { - lcd_setstatuspgm(message); - lcd_status_message_level = 1; - lcd_return_to_status(); + if (severity > lcd_status_message_level) { + lcd_setstatuspgm(message); + lcd_status_message_level = severity; + lcd_return_to_status(); + } } -void lcd_setalertstatus(const char* message) +void lcd_setalertstatus(const char* message, uint8_t severity) { - lcd_setstatus(message); - lcd_status_message_level = 1; - lcd_return_to_status(); + if (severity > lcd_status_message_level) { + lcd_setstatus(message); + lcd_status_message_level = severity; + lcd_return_to_status(); + } } void lcd_reset_alert_level() diff --git a/Firmware/ultralcd.h b/Firmware/ultralcd.h index 68b933d02..e3683576e 100755 --- a/Firmware/ultralcd.h +++ b/Firmware/ultralcd.h @@ -12,13 +12,20 @@ extern void menu_lcd_lcdupdate_func(void); void ultralcd_init(); void lcd_setstatus(const char* message); void lcd_setstatuspgm(const char* message); + +//! LCD status severities +#define LCD_STATUS_CRITICAL 2 //< Heater failure +#define LCD_STATUS_ALERT 1 //< Other hardware issue +#define LCD_STATUS_NONE 0 //< No alert message set + //! return to the main status screen and display the alert message //! Beware - it has sideeffects: //! - always returns the display to the main status screen //! - always makes lcd_reset (which is slow and causes flicker) -//! - does not update the message if there is already one (i.e. lcd_status_message_level > 0) -void lcd_setalertstatus(const char* message); -void lcd_setalertstatuspgm(const char* message); +//! - does not update the message if there is one with the same (or higher) severity present +void lcd_setalertstatus(const char* message, uint8_t severity = LCD_STATUS_ALERT); +void lcd_setalertstatuspgm(const char* message, uint8_t severity = LCD_STATUS_ALERT); + //! only update the alert message on the main status screen //! has no sideeffects, may be called multiple times void lcd_updatestatus(const char *message); From 57abffda1b1810048bbc5647a928acc56deb1675 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Tue, 7 Dec 2021 11:09:58 +0100 Subject: [PATCH 02/10] Update temperature-related error message to use LCD_STATUS_CRITICAL --- Firmware/temperature.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Firmware/temperature.cpp b/Firmware/temperature.cpp index 3765589b8..c753db194 100755 --- a/Firmware/temperature.cpp +++ b/Firmware/temperature.cpp @@ -1483,13 +1483,12 @@ uint8_t last_alert_sent_to_lcd = LCDALERT_NONE; //! update the current temperature error message //! @param type short error abbreviation (PROGMEM) -//! @param func optional lcd update function (lcd_setalertstatus when first setting the error) -void temp_update_messagepgm(const char* PROGMEM type, void (*func)(const char*) = lcd_updatestatus) +void temp_update_messagepgm(const char* PROGMEM type) { char msg[LCD_WIDTH]; strcpy_P(msg, PSTR("Err: ")); strcat_P(msg, type); - (*func)(msg); + lcd_setalertstatus(msg, LCD_STATUS_CRITICAL); } //! signal a temperature error on both the lcd and serial @@ -1497,7 +1496,7 @@ void temp_update_messagepgm(const char* PROGMEM type, void (*func)(const char*) //! @param e optional extruder index for hotend errors void temp_error_messagepgm(const char* PROGMEM type, uint8_t e = EXTRUDERS) { - temp_update_messagepgm(type, lcd_setalertstatus); + temp_update_messagepgm(type); SERIAL_ERROR_START; From a3915b57b9f7984164876a7ba70c1c8196b7556f Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Tue, 7 Dec 2021 11:11:06 +0100 Subject: [PATCH 03/10] Improve temp_runaway_stop robustness Remove most of the duplicated code inside temp_runaway_stop(), making it identical to the other temperature handlers. Move the lower-level functions required to stop the entirety of the machine into UnconditionalStop(). Reuse this function inside lcd_print_stop(). Set the LCD alert message before calling Stop(), as done in other safety handlers, so that the error is visible while the printer is stopping. This also avoids other temporary status messages to appear before the real issue is shown and/or STEALING the first CRITICAL alert level before we do. --- Firmware/Marlin.h | 6 ++++-- Firmware/Marlin_main.cpp | 37 +++++++++++++++++++++++++++++++++++++ Firmware/temperature.cpp | 37 ++++++++++--------------------------- Firmware/temperature.h | 2 +- Firmware/ultralcd.cpp | 25 +++++++------------------ 5 files changed, 59 insertions(+), 48 deletions(-) diff --git a/Firmware/Marlin.h b/Firmware/Marlin.h index c2c5ca933..85c4dae10 100755 --- a/Firmware/Marlin.h +++ b/Firmware/Marlin.h @@ -236,10 +236,12 @@ void update_currents(); void get_coordinates(); void prepare_move(); void kill(const char *full_screen_message = NULL, unsigned char id = 0); -void Stop(); -bool IsStopped(); void finishAndDisableSteppers(); +void UnconditionalStop(); // Stop heaters, motion and clear current print status +void Stop(); // Emergency stop used by overtemp functions which allows recovery +bool IsStopped(); // Returns true if the print has been stopped + //put an ASCII command at the end of the current buffer, read from flash #define enquecommand_P(cmd) enquecommand(cmd, true) diff --git a/Firmware/Marlin_main.cpp b/Firmware/Marlin_main.cpp index 2d53b1cd9..5fb82577d 100755 --- a/Firmware/Marlin_main.cpp +++ b/Firmware/Marlin_main.cpp @@ -10154,6 +10154,31 @@ void kill(const char *full_screen_message, unsigned char id) } // Wait for reset } +void UnconditionalStop() +{ + CRITICAL_SECTION_START; + + // Disable all heaters and unroll the temperature wait loop stack + disable_heater(); + + // Clear any saved printing state + cancel_saved_printing(); + + // Abort the planner + planner_abort_hard(); + + // Reset the queue + cmdqueue_reset(); + cmdqueue_serial_disabled = false; + + // Reset the sd status + card.sdprinting = false; + card.closefile(); + + st_reset_timer(); + CRITICAL_SECTION_END; +} + // Stop: Emergency stop used by overtemp functions which allows recovery // // In addition to stopping the print, this prevents subsequent G[0-3] commands to be @@ -10166,15 +10191,27 @@ void kill(const char *full_screen_message, unsigned char id) // the addition of disabling the headers) could allow true recovery in the future. void Stop() { + // Keep disabling heaters disable_heater(); + + // Call the regular stop function if that's the first time during a new print if(Stopped == false) { Stopped = true; lcd_print_stop(); Stopped_gcode_LastN = gcode_LastN; // Save last g_code for restart + + // Eventually report the stopped status (though this is usually overridden by a + // higher-priority alert status message) SERIAL_ERROR_START; SERIAL_ERRORLNRPGM(MSG_ERR_STOPPED); LCD_MESSAGERPGM(_T(MSG_STOPPED)); } + + // Return to the status screen to stop any pending menu action which could have been + // started by the user while stuck in the Stopped state. This also ensures the NEW + // error is immediately shown. + if (menu_menu != lcd_status_screen) + lcd_return_to_status(); } bool IsStopped() { return Stopped; }; diff --git a/Firmware/temperature.cpp b/Firmware/temperature.cpp index c753db194..f84d966e0 100755 --- a/Firmware/temperature.cpp +++ b/Firmware/temperature.cpp @@ -1381,33 +1381,15 @@ void temp_runaway_check(int _heater_id, float _target_temperature, float _curren void temp_runaway_stop(bool isPreheat, bool isBed) { - cancel_heatup = true; - quickStop(); - if (card.sdprinting) + disable_heater(); + Sound_MakeCustom(200,0,true); + + if (isPreheat) { - card.sdprinting = false; - card.closefile(); - } - // Clean the input command queue - // This is necessary, because in command queue there can be commands which would later set heater or bed temperature. - cmdqueue_reset(); - - disable_heater(); - disable_x(); - disable_y(); - disable_e0(); - disable_e1(); - disable_e2(); - manage_heater(); - lcd_update(0); - Sound_MakeCustom(200,0,true); - - if (isPreheat) - { - Stop(); - isBed ? LCD_ALERTMESSAGEPGM("BED PREHEAT ERROR") : LCD_ALERTMESSAGEPGM("PREHEAT ERROR"); + lcd_setalertstatuspgm(isBed? PSTR("BED PREHEAT ERROR") : PSTR("PREHEAT ERROR"), LCD_STATUS_CRITICAL); SERIAL_ERROR_START; isBed ? SERIAL_ERRORLNPGM(" THERMAL RUNAWAY ( PREHEAT HEATBED)") : SERIAL_ERRORLNPGM(" THERMAL RUNAWAY ( PREHEAT HOTEND)"); + #ifdef EXTRUDER_ALTFAN_DETECT altfanStatus.altfanOverride = 1; //full speed #endif //EXTRUDER_ALTFAN_DETECT @@ -1418,22 +1400,23 @@ void temp_runaway_stop(bool isPreheat, bool isBed) #else //FAN_SOFT_PWM analogWrite(FAN_PIN, 255); #endif //FAN_SOFT_PWM - fanSpeed = 255; - delayMicroseconds(2000); } else { - isBed ? LCD_ALERTMESSAGEPGM("BED THERMAL RUNAWAY") : LCD_ALERTMESSAGEPGM("THERMAL RUNAWAY"); + lcd_setalertstatuspgm(isBed? PSTR("BED THERMAL RUNAWAY") : PSTR("THERMAL RUNAWAY"), LCD_STATUS_CRITICAL); SERIAL_ERROR_START; isBed ? SERIAL_ERRORLNPGM(" HEATBED THERMAL RUNAWAY") : SERIAL_ERRORLNPGM(" HOTEND THERMAL RUNAWAY"); } + + Stop(); } #endif void disable_heater() { + cancel_heatup = true; setAllTargetHotends(0); setTargetBed(0); #if defined(TEMP_0_PIN) && TEMP_0_PIN > -1 diff --git a/Firmware/temperature.h b/Firmware/temperature.h index 0bf944724..a2d1db2bd 100755 --- a/Firmware/temperature.h +++ b/Firmware/temperature.h @@ -220,7 +220,7 @@ FORCE_INLINE bool isCoolingBed() { #define CHECK_ALL_HEATERS (checkAllHotends()||(target_temperature_bed!=0)) int getHeaterPower(int heater); -void disable_heater(); +void disable_heater(); // Disable all heaters and unroll the temperature wait loop stack void updatePID(); diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index fcce9b199..782fe7b3c 100755 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -7089,25 +7089,15 @@ static void lcd_sd_updir() void lcd_print_stop() { - if (!card.sdprinting) { - SERIAL_ECHOLNRPGM(MSG_OCTOPRINT_CANCEL); // for Octoprint + if (!IsStopped()) { + // Stop the print if we didn't already due to an error + if (!card.sdprinting) { + SERIAL_ECHOLNRPGM(MSG_OCTOPRINT_CANCEL); // for Octoprint + } + UnconditionalStop(); } - cmdqueue_serial_disabled = false; //for when canceling a print with a fancheck - - CRITICAL_SECTION_START; - - // Clear any saved printing state - cancel_saved_printing(); - - // Abort the planner/queue/sd - planner_abort_hard(); - cmdqueue_reset(); - card.sdprinting = false; - card.closefile(); - st_reset_timer(); - - CRITICAL_SECTION_END; + // TODO: all the following should be moved in the main marlin loop! #ifdef MESH_BED_LEVELING mbl.active = false; //also prevents undoing the mbl compensation a second time in the second planner_abort_hard() #endif @@ -7122,7 +7112,6 @@ void lcd_print_stop() lcd_commands_type = LcdCommands::Idle; lcd_cooldown(); //turns off heaters and fan; goes to status screen. - cancel_heatup = true; //unroll temperature wait loop stack. current_position[Z_AXIS] += 10; //lift Z. plan_buffer_line_curposXYZE(manual_feedrate[Z_AXIS] / 60); From 36a7b5ca569ba82b2c4d7d4c7c2686c82a3193e0 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Tue, 7 Dec 2021 11:21:51 +0100 Subject: [PATCH 04/10] Avoid redundant checks in lcd_setalertstatus* --- Firmware/ultralcd.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index 782fe7b3c..81f0097c8 100755 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -8805,7 +8805,7 @@ void lcd_updatestatus(const char *message){ void lcd_setalertstatuspgm(const char* message, uint8_t severity) { if (severity > lcd_status_message_level) { - lcd_setstatuspgm(message); + lcd_updatestatuspgm(message); lcd_status_message_level = severity; lcd_return_to_status(); } @@ -8814,7 +8814,7 @@ void lcd_setalertstatuspgm(const char* message, uint8_t severity) void lcd_setalertstatus(const char* message, uint8_t severity) { if (severity > lcd_status_message_level) { - lcd_setstatus(message); + lcd_updatestatus(message); lcd_status_message_level = severity; lcd_return_to_status(); } From 7ff117d0c4d2ba03772d6a8244e004e7fedda94c Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Tue, 7 Dec 2021 11:35:35 +0100 Subject: [PATCH 05/10] temp_runaway_stop: remove spourious space in error message --- Firmware/temperature.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Firmware/temperature.cpp b/Firmware/temperature.cpp index f84d966e0..c53242aef 100755 --- a/Firmware/temperature.cpp +++ b/Firmware/temperature.cpp @@ -1388,7 +1388,7 @@ void temp_runaway_stop(bool isPreheat, bool isBed) { lcd_setalertstatuspgm(isBed? PSTR("BED PREHEAT ERROR") : PSTR("PREHEAT ERROR"), LCD_STATUS_CRITICAL); SERIAL_ERROR_START; - isBed ? SERIAL_ERRORLNPGM(" THERMAL RUNAWAY ( PREHEAT HEATBED)") : SERIAL_ERRORLNPGM(" THERMAL RUNAWAY ( PREHEAT HOTEND)"); + isBed ? SERIAL_ERRORLNPGM(" THERMAL RUNAWAY (PREHEAT HEATBED)") : SERIAL_ERRORLNPGM(" THERMAL RUNAWAY (PREHEAT HOTEND)"); #ifdef EXTRUDER_ALTFAN_DETECT altfanStatus.altfanOverride = 1; //full speed From 78f856c8d6dd0dbc7d259f45679a8b85d7c2cb01 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Tue, 7 Dec 2021 16:56:39 +0100 Subject: [PATCH 06/10] Do not unconditionally overwrite the status message in check_file() No function should touch the status message directly without checking the message severity level first. Replace the strcpy_P with lcd_setstatuspgm(). --- Firmware/ultralcd.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index 81f0097c8..e88e2285e 100755 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -8593,7 +8593,7 @@ static bool check_file(const char* filename) { cmdqueue_serial_disabled = false; card.printingHasFinished(); - strncpy_P(lcd_status_message, _T(WELCOME_MSG), LCD_WIDTH); + lcd_setstatuspgm(_T(WELCOME_MSG)); lcd_finishstatus(); return result; } From 83693bf4ccea3bfec23ef38e9478e4da6dae1363 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Tue, 7 Dec 2021 17:42:27 +0100 Subject: [PATCH 07/10] Remove useless assignment in lcd_cooldown The current filament action will be aborted by lcd_return_to_status() --- Firmware/ultralcd.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index e88e2285e..bc35e6a0c 100755 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -1399,7 +1399,6 @@ static void lcd_cooldown() setAllTargetHotends(0); setTargetBed(0); fanSpeed = 0; - eFilamentAction = FilamentAction::None; lcd_return_to_status(); } From 32d8d892f5792bc5d3f8ced45c9da63c8ca73972 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Fri, 10 Dec 2021 01:43:59 +0100 Subject: [PATCH 08/10] Actually call UnconditionalStop() in Stop()->lcd_print_stop() Remove incorrect check introduced during development. --- Firmware/ultralcd.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index bc35e6a0c..50bdeb970 100755 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -7088,13 +7088,10 @@ static void lcd_sd_updir() void lcd_print_stop() { - if (!IsStopped()) { - // Stop the print if we didn't already due to an error - if (!card.sdprinting) { - SERIAL_ECHOLNRPGM(MSG_OCTOPRINT_CANCEL); // for Octoprint - } - UnconditionalStop(); + if (!card.sdprinting) { + SERIAL_ECHOLNRPGM(MSG_OCTOPRINT_CANCEL); // for Octoprint } + UnconditionalStop(); // TODO: all the following should be moved in the main marlin loop! #ifdef MESH_BED_LEVELING From 320835a1b7b29a7f3e01710e346dd0895600e0e5 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Fri, 10 Dec 2021 01:46:20 +0100 Subject: [PATCH 09/10] Do not cancel wait-for-temperature loops in disable_heaters() Partially revert previous change: cancelling a single loop would often advance the gcode stream to the next wait-for loop if executed at the beginning of an SD print, implicitly turning off the flag again. Cancel the loop directly in UnconditionalStop() which stops the command queue as well in an atomic way, handling this correctly. --- Firmware/Marlin_main.cpp | 3 ++- Firmware/temperature.cpp | 1 - Firmware/temperature.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Firmware/Marlin_main.cpp b/Firmware/Marlin_main.cpp index 5fb82577d..30c205895 100755 --- a/Firmware/Marlin_main.cpp +++ b/Firmware/Marlin_main.cpp @@ -294,7 +294,7 @@ uint8_t newFanSpeed = 0; bool powersupply = true; #endif -bool cancel_heatup = false ; +bool cancel_heatup = false; int8_t busy_state = NOT_BUSY; static long prev_busy_signal_ms = -1; @@ -10160,6 +10160,7 @@ void UnconditionalStop() // Disable all heaters and unroll the temperature wait loop stack disable_heater(); + cancel_heatup = true; // Clear any saved printing state cancel_saved_printing(); diff --git a/Firmware/temperature.cpp b/Firmware/temperature.cpp index c53242aef..87ffc6149 100755 --- a/Firmware/temperature.cpp +++ b/Firmware/temperature.cpp @@ -1416,7 +1416,6 @@ void temp_runaway_stop(bool isPreheat, bool isBed) void disable_heater() { - cancel_heatup = true; setAllTargetHotends(0); setTargetBed(0); #if defined(TEMP_0_PIN) && TEMP_0_PIN > -1 diff --git a/Firmware/temperature.h b/Firmware/temperature.h index a2d1db2bd..b0e035d78 100755 --- a/Firmware/temperature.h +++ b/Firmware/temperature.h @@ -220,7 +220,7 @@ FORCE_INLINE bool isCoolingBed() { #define CHECK_ALL_HEATERS (checkAllHotends()||(target_temperature_bed!=0)) int getHeaterPower(int heater); -void disable_heater(); // Disable all heaters and unroll the temperature wait loop stack +void disable_heater(); // Disable all heaters void updatePID(); From 3849f9785a98c5454531a85484c589f637b86260 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Fri, 10 Dec 2021 01:51:43 +0100 Subject: [PATCH 10/10] Make cancel_heatup also abort cooldown in M190 This matches the expected behavior, as already implemented in wait_for_heater(). --- Firmware/Marlin_main.cpp | 2 +- Firmware/ultralcd.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Firmware/Marlin_main.cpp b/Firmware/Marlin_main.cpp index 30c205895..527df0a7e 100755 --- a/Firmware/Marlin_main.cpp +++ b/Firmware/Marlin_main.cpp @@ -6751,7 +6751,7 @@ Sigma_Exit: target_direction = isHeatingBed(); // true if heating, false if cooling KEEPALIVE_STATE(NOT_BUSY); - while ( (target_direction)&&(!cancel_heatup) ? (isHeatingBed()) : (isCoolingBed()&&(CooldownNoWait==false)) ) + while ( (!cancel_heatup) && (target_direction ? (isHeatingBed()) : (isCoolingBed()&&(CooldownNoWait==false))) ) { if(( _millis() - codenum) > 1000 ) //Print Temp Reading every 1 second while heating up. { diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index 50bdeb970..daac8d71c 100755 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -7104,6 +7104,7 @@ void lcd_print_stop() pause_time = 0; save_statistics(total_filament_used, t); + // reset current command lcd_commands_step = 0; lcd_commands_type = LcdCommands::Idle;