From f96cc2f400cde3d46816875332de32463f1271ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0ni=20M=C3=A1r=20Gilbert?= Date: Sun, 28 Jul 2024 18:20:17 +0000 Subject: [PATCH 1/2] cleanup recursion workaround in nozzle change This fix is no longer needed today. LCD knob clicks / and rotation, take care of updating lcd_draw_update. The real bug was likely lcd_show_multiscreen_message_yes_no_and_wait_P calling lcd_update(), this is fixed now since. --- Firmware/ultralcd.cpp | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index 08ca782ad..629c8801e 100644 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -984,10 +984,7 @@ void lcd_commands() break; case 3: #ifndef QUICK_NOZZLE_CHANGE - lcd_update_enabled = false; //hack to avoid lcd_update recursion. lcd_show_fullscreen_message_and_wait_P(_T(MSG_NOZZLE_CNG_READ_HELP)); - lcd_update_enabled = true; - lcd_draw_update = 2; //force lcd clear and update after the stack unwinds. enquecommand_P(G28W); enquecommand_P(PSTR("G1 X125 Z200 F1000")); enquecommand_P(PSTR("M109 S280")); @@ -1000,14 +997,9 @@ void lcd_commands() fanSpeed = 255; //turn on fan disable_heater(); uint8_t choice = lcd_show_multiscreen_message_yes_no_and_wait_P(_T(MSG_NOZZLE_CNG_COOLDOWN), true, LCD_LEFT_BUTTON_CHOICE); - lcd_update_enabled = false; //hack to avoid lcd_update recursion. if (choice == LCD_MIDDLE_BUTTON_CHOICE) { - lcd_update_enabled = true; - lcd_draw_update = 2; //force lcd clear and update after the stack unwinds. break; } - lcd_update_enabled = true; - lcd_draw_update = 2; //force lcd clear and update after the stack unwinds. } enquecommand_P(G28W); //home enquecommand_P(PSTR("G1 X125 Z200 F1000")); //move to top center @@ -1016,7 +1008,6 @@ void lcd_commands() break; case 2: enquecommand_P(PSTR("M84 XY")); - lcd_update_enabled = false; //hack to avoid lcd_update recursion. if (lcd_show_multiscreen_message_yes_no_and_wait_P(_T(MSG_NOZZLE_CNG_CHANGED), false) == LCD_LEFT_BUTTON_CHOICE) { #ifndef QUICK_NOZZLE_CHANGE setTargetHotend(0); @@ -1028,7 +1019,6 @@ void lcd_commands() #endif //QUICK_NOZZLE_CHANGE lcd_commands_step = 1; } - lcd_update_enabled = true; break; case 1: lcd_commands_step = 0; From 25a11bb7d78d8ff6f2e9a5d6595a05bc6b67bb14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0ni=20M=C3=A1r=20Gilbert?= Date: Wed, 31 Jul 2024 12:27:38 +0000 Subject: [PATCH 2/2] Forbid LcdUpdateDisabler to call lcd_update() The fixes a scenario where: lcd_status_screen() calls lcd_commands() upon exiting lcd_show_fullscreen_message_and_wait_P(_T(MSG_NOZZLE_CNG_READ_HELP)); and so not allowing the user to leave the screen since it will keep being rendered endlessly. This change only affects lcd_show_fullscreen_message_and_wait_P --- Firmware/lcd.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Firmware/lcd.h b/Firmware/lcd.h index 897195070..031887cc0 100644 --- a/Firmware/lcd.h +++ b/Firmware/lcd.h @@ -111,18 +111,18 @@ extern void lcd_buttons_update(void); //! When constructed (on stack), original state state of lcd_update_enabled is stored //! and LCD updates are disabled. //! When destroyed (gone out of scope), original state of LCD update is restored. -//! It has zero overhead compared to storing bool saved = lcd_update_enabled -//! and calling lcd_update_enable(false) and lcd_update_enable(saved). +//! Do not call lcd_update_enable() to prevent calling lcd_update() in sensitive code. +//! in certain scenarios it will cause recursion e.g. in the menus. class LcdUpdateDisabler { public: LcdUpdateDisabler(): m_updateEnabled(lcd_update_enabled) { - lcd_update_enable(false); + lcd_update_enabled = false; } ~LcdUpdateDisabler() { - lcd_update_enable(m_updateEnabled); + lcd_update_enabled = m_updateEnabled; } private: