From bfdef3af9fc6cbdf0b08d83a1200582a511dcd47 Mon Sep 17 00:00:00 2001 From: Alex Voinea Date: Mon, 10 Apr 2023 11:42:34 +0200 Subject: [PATCH 1/5] Fix menu items optimizations when extracting to functions. Properly increment menu_item when the menu item is clicked so that you don't get multiple menu items clicked if in view (when the menu code is deduplicated to functions) --- Firmware/menu.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Firmware/menu.cpp b/Firmware/menu.cpp index db7d9c293..ee7d147b1 100755 --- a/Firmware/menu.cpp +++ b/Firmware/menu.cpp @@ -140,6 +140,7 @@ void menu_submenu_no_reset(menu_func_t submenu, const bool feedback) uint8_t menu_item_ret(void) { lcd_draw_update = 2; + menu_item++; return 1; } From d3d201730e7b42fe48c717e07dd563fa86aa29c9 Mon Sep 17 00:00:00 2001 From: Alex Voinea Date: Mon, 10 Apr 2023 11:54:51 +0200 Subject: [PATCH 2/5] Do not return early if the menu item is clicked Just let the menu draw till the end even if an item is clicked. The worst this can do is waste some clock cycles flash: -1222B ram: 0B --- Firmware/menu.h | 22 +++++++++++----------- Firmware/ultralcd.cpp | 6 +++--- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/Firmware/menu.h b/Firmware/menu.h index c3cf80924..a0a6f845f 100755 --- a/Firmware/menu.h +++ b/Firmware/menu.h @@ -92,36 +92,36 @@ extern uint8_t menu_item_ret(void); #define MENU_ITEM_DUMMY() menu_item_dummy() extern void menu_item_dummy(void); -#define MENU_ITEM_TEXT_P(str) do { if (menu_item_text_P(str)) return; } while (0) +#define MENU_ITEM_TEXT_P(str) do { menu_item_text_P(str); } while (0) extern uint8_t menu_item_text_P(const char* str); -#define MENU_ITEM_SUBMENU_P(str, submenu) do { if (menu_item_submenu_P(str, submenu)) return; } while (0) +#define MENU_ITEM_SUBMENU_P(str, submenu) do { menu_item_submenu_P(str, submenu); } while (0) extern uint8_t menu_item_submenu_P(const char* str, menu_func_t submenu); -#define MENU_ITEM_SUBMENU_E(sheet, submenu) do { if (menu_item_submenu_E(sheet, submenu)) return; } while (0) +#define MENU_ITEM_SUBMENU_E(sheet, submenu) do { menu_item_submenu_E(sheet, submenu); } while (0) extern uint8_t menu_item_submenu_E(const Sheet &sheet, menu_func_t submenu); -#define MENU_ITEM_FUNCTION_E(sheet, submenu) do { if (menu_item_function_E(sheet, submenu)) return; } while (0) +#define MENU_ITEM_FUNCTION_E(sheet, submenu) do { menu_item_function_E(sheet, submenu); } while (0) extern uint8_t menu_item_function_E(const Sheet &sheet, menu_func_t func); -#define MENU_ITEM_BACK_P(str) do { if (menu_item_back_P(str)) return; } while (0) +#define MENU_ITEM_BACK_P(str) do { menu_item_back_P(str); } while (0) extern uint8_t menu_item_back_P(const char* str); // leaving menu - this condition must be immediately before MENU_ITEM_BACK_P #define ON_MENU_LEAVE(func) do { if (menu_item_leave()){ func } } while (0) extern bool menu_item_leave(); -#define MENU_ITEM_FUNCTION_P(str, func) do { if (menu_item_function_P(str, func)) return; } while (0) +#define MENU_ITEM_FUNCTION_P(str, func) do { menu_item_function_P(str, func); } while (0) extern uint8_t menu_item_function_P(const char* str, menu_func_t func); -#define MENU_ITEM_FUNCTION_NR_P(str, number, func, fn_par) do { if (menu_item_function_P(str, number, func, fn_par)) return; } while (0) +#define MENU_ITEM_FUNCTION_NR_P(str, number, func, fn_par) do { menu_item_function_P(str, number, func, fn_par); } while (0) extern uint8_t menu_item_function_P(const char* str, char number, void (*func)(uint8_t), uint8_t fn_par); -#define MENU_ITEM_TOGGLE_P(str, toggle, func) do { if (menu_item_toggle_P(str, toggle, func, 0x02)) return; } while (0) -#define MENU_ITEM_TOGGLE(str, toggle, func) do { if (menu_item_toggle_P(str, toggle, func, 0x00)) return; } while (0) +#define MENU_ITEM_TOGGLE_P(str, toggle, func) do { menu_item_toggle_P(str, toggle, func, 0x02); } while (0) +#define MENU_ITEM_TOGGLE(str, toggle, func) do { menu_item_toggle_P(str, toggle, func, 0x00); } while (0) extern uint8_t menu_item_toggle_P(const char* str, const char* toggle, menu_func_t func, const uint8_t settings); -#define MENU_ITEM_GCODE_P(str, str_gcode) do { if (menu_item_gcode_P(str, str_gcode)) return; } while (0) +#define MENU_ITEM_GCODE_P(str, str_gcode) do { menu_item_gcode_P(str, str_gcode); } while (0) extern uint8_t menu_item_gcode_P(const char* str, const char* str_gcode); @@ -142,7 +142,7 @@ struct SheetFormatBuffer extern void menu_format_sheet_E(const Sheet &sheet_E, SheetFormatBuffer &buffer); -#define MENU_ITEM_EDIT_int3_P(str, pval, minval, maxval) do { if (menu_item_edit_P(str, pval, minval, maxval)) return; } while (0) +#define MENU_ITEM_EDIT_int3_P(str, pval, minval, maxval) do { menu_item_edit_P(str, pval, minval, maxval); } while (0) //#define MENU_ITEM_EDIT_int3_P(str, pval, minval, maxval) MENU_ITEM_EDIT(int3, str, pval, minval, maxval) template extern uint8_t menu_item_edit_P(const char* str, T pval, int16_t min_val, int16_t max_val); diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index 1585b2298..cd86a3388 100644 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -287,8 +287,8 @@ static void lcd_implementation_drawmenu_sddirectory(uint8_t row, const char* lon -#define MENU_ITEM_SDDIR(str_fn, str_fnl) do { if (menu_item_sddir(str_fn, str_fnl)) return; } while (0) -#define MENU_ITEM_SDFILE(str_fn, str_fnl) do { if (menu_item_sdfile(str_fn, str_fnl)) return; } while (0) +#define MENU_ITEM_SDDIR(str_fn, str_fnl) do { menu_item_sddir(str_fn, str_fnl); } while (0) +#define MENU_ITEM_SDFILE(str_fn, str_fnl) do { menu_item_sdfile(str_fn, str_fnl); } while (0) uint8_t menu_item_sddir(const char* str_fn, char* str_fnl) @@ -5470,7 +5470,7 @@ static uint8_t lcd_advance_K() return 0; } -#define MENU_ITEM_EDIT_advance_K() do { if (lcd_advance_K()) return; } while (0) +#define MENU_ITEM_EDIT_advance_K() do { lcd_advance_K(); } while (0) #endif From 9a1eb7b2395a05e182340bad0b6808b02da0fa93 Mon Sep 17 00:00:00 2001 From: Alex Voinea Date: Mon, 10 Apr 2023 12:58:38 +0200 Subject: [PATCH 3/5] Prevent rendering of the remaining menu items if the menu is clicked Also prevent clicking --- Firmware/menu.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Firmware/menu.cpp b/Firmware/menu.cpp index ee7d147b1..c76c0ce23 100755 --- a/Firmware/menu.cpp +++ b/Firmware/menu.cpp @@ -77,6 +77,11 @@ void menu_start(void) void menu_end(void) { + if (menu_row >= LCD_HEIGHT) + { + // Early abort if the menu was clicked. The current menu might have changed because of the click event + return; + } if (lcd_encoder >= menu_item) { lcd_encoder = menu_item - 1; @@ -141,6 +146,11 @@ uint8_t menu_item_ret(void) { lcd_draw_update = 2; menu_item++; + + //prevent the rest of the menu items from rendering or getting clicked + menu_row = LCD_HEIGHT; // early exit from the MENU_BEGIN() for loop at the end of the current cycle + menu_line = 0; // prevent subsequent menu items from rendering at all in the current MENU_BEGIN() for loop cycle + menu_clicked = 0; // prevent subsequent items from being able to be clicked in case the current menu or position was changed by the clicked menu item return 1; } From b735c3d040c46b95f7634d630ff4ea0e411999e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0ni=20M=C3=A1r=20Gilbert?= Date: Sun, 19 Feb 2023 21:39:22 +0000 Subject: [PATCH 4/5] optimisation: extract common code into SETTINGS_FANS_CHECK Implement it as a function instead of preprocessor macro this allows us to control inlining Change in memory: Flash: -34 bytes SRAM: 0 bytes --- Firmware/ultralcd.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index cd86a3388..37eb99bf9 100644 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -4211,6 +4211,10 @@ static void SETTINGS_SILENT_MODE() { } } +void SETTINGS_FANS_CHECK() { + MENU_ITEM_TOGGLE_P(_T(MSG_FANS_CHECK), fans_check_enabled ? _T(MSG_ON) : _T(MSG_OFF), lcd_set_fan_check); +} + #ifndef MMU_FORCE_STEALTH_MODE #define SETTINGS_MMU_MODE \ do\ @@ -4557,7 +4561,7 @@ static void lcd_settings_menu() MENU_ITEM_FUNCTION_P(PSTR("Reset MMU"), mmu_reset); } - MENU_ITEM_TOGGLE_P(_T(MSG_FANS_CHECK), fans_check_enabled ? _T(MSG_ON) : _T(MSG_OFF), lcd_set_fan_check); + SETTINGS_FANS_CHECK(); SETTINGS_SILENT_MODE(); if(!farm_mode) @@ -5528,7 +5532,7 @@ static void lcd_tune_menu() SETTINGS_CUTTER; } - MENU_ITEM_TOGGLE_P(_T(MSG_FANS_CHECK), fans_check_enabled ? _T(MSG_ON) : _T(MSG_OFF), lcd_set_fan_check); + SETTINGS_FANS_CHECK(); SETTINGS_SILENT_MODE(); SETTINGS_MMU_MODE; SETTINGS_SOUND; From d688f6ec6ff28e3c1bcfd7611a451c33b92bcda2 Mon Sep 17 00:00:00 2001 From: Alex Voinea Date: Mon, 10 Apr 2023 15:29:58 +0200 Subject: [PATCH 5/5] Menu item code: optimize return --- Firmware/menu.cpp | 67 ++++++++++++++++++++++--------------------- Firmware/menu.h | 20 ++++++------- Firmware/ultralcd.cpp | 18 ++++++------ 3 files changed, 53 insertions(+), 52 deletions(-) diff --git a/Firmware/menu.cpp b/Firmware/menu.cpp index c76c0ce23..9e262edec 100755 --- a/Firmware/menu.cpp +++ b/Firmware/menu.cpp @@ -142,7 +142,7 @@ void menu_submenu_no_reset(menu_func_t submenu, const bool feedback) } } -uint8_t menu_item_ret(void) +void menu_item_ret(void) { lcd_draw_update = 2; menu_item++; @@ -151,7 +151,6 @@ uint8_t menu_item_ret(void) menu_row = LCD_HEIGHT; // early exit from the MENU_BEGIN() for loop at the end of the current cycle menu_line = 0; // prevent subsequent menu items from rendering at all in the current MENU_BEGIN() for loop cycle menu_clicked = 0; // prevent subsequent items from being able to be clicked in case the current menu or position was changed by the clicked menu item - return 1; } static char menu_selection_mark(){ @@ -258,13 +257,16 @@ uint8_t menu_item_text_P(const char* str) { if (lcd_draw_update) menu_draw_item_puts_P(' ', str); if (menu_clicked && (lcd_encoder == menu_item)) - return menu_item_ret(); + { + menu_item_ret(); + return 1; + } } menu_item++; return 0; } -uint8_t menu_item_submenu_P(const char* str, menu_func_t submenu) +void menu_item_submenu_P(const char* str, menu_func_t submenu) { if (menu_item == menu_line) { @@ -272,14 +274,14 @@ uint8_t menu_item_submenu_P(const char* str, menu_func_t submenu) if (menu_clicked && (lcd_encoder == menu_item)) { menu_submenu(submenu); - return menu_item_ret(); + menu_item_ret(); + return; } } menu_item++; - return 0; } -uint8_t menu_item_submenu_E(const Sheet &sheet, menu_func_t submenu) +void menu_item_submenu_E(const Sheet &sheet, menu_func_t submenu) { if (menu_item == menu_line) { @@ -287,14 +289,14 @@ uint8_t menu_item_submenu_E(const Sheet &sheet, menu_func_t submenu) if (menu_clicked && (lcd_encoder == menu_item)) { menu_submenu(submenu); - return menu_item_ret(); + menu_item_ret(); + return; } } menu_item++; - return 0; } -uint8_t __attribute__((noinline)) menu_item_function_E(const Sheet &sheet, menu_func_t func) +void __attribute__((noinline)) menu_item_function_E(const Sheet &sheet, menu_func_t func) { if (menu_item == menu_line) { @@ -304,14 +306,14 @@ uint8_t __attribute__((noinline)) menu_item_function_E(const Sheet &sheet, menu_ lcd_update_enabled = 0; if (func) func(); lcd_update_enabled = 1; - return menu_item_ret(); + menu_item_ret(); + return; } } menu_item++; - return 0; } -uint8_t menu_item_back_P(const char* str) +void menu_item_back_P(const char* str) { if (menu_item == menu_line) { @@ -319,18 +321,18 @@ uint8_t menu_item_back_P(const char* str) if (menu_clicked && (lcd_encoder == menu_item)) { menu_back(); - return menu_item_ret(); + menu_item_ret(); + return; } } menu_item++; - return 0; } bool __attribute__((noinline)) menu_item_leave(){ return ((menu_item == menu_line) && menu_clicked && (lcd_encoder == menu_item)) || menu_leaving; } -uint8_t menu_item_function_P(const char* str, menu_func_t func) +void menu_item_function_P(const char* str, menu_func_t func) { if (menu_item == menu_line) { @@ -340,11 +342,11 @@ uint8_t menu_item_function_P(const char* str, menu_func_t func) lcd_update_enabled = 0; if (func) func(); lcd_update_enabled = 1; - return menu_item_ret(); + menu_item_ret(); + return; } } menu_item++; - return 0; } //! @brief Menu item function taking single parameter @@ -356,7 +358,7 @@ uint8_t menu_item_function_P(const char* str, menu_func_t func) //! @param fn_par value to be passed to function //! @retval 0 //! @retval 1 Item was clicked -uint8_t menu_item_function_P(const char* str, char number, void (*func)(uint8_t), uint8_t fn_par) +void menu_item_function_P(const char* str, char number, void (*func)(uint8_t), uint8_t fn_par) { if (menu_item == menu_line) { @@ -366,14 +368,14 @@ uint8_t menu_item_function_P(const char* str, char number, void (*func)(uint8_t) lcd_update_enabled = 0; if (func) func(fn_par); lcd_update_enabled = 1; - return menu_item_ret(); + menu_item_ret(); + return; } } menu_item++; - return 0; } -uint8_t menu_item_toggle_P(const char* str, const char* toggle, menu_func_t func, const uint8_t settings) +void menu_item_toggle_P(const char* str, const char* toggle, menu_func_t func, const uint8_t settings) { if (menu_item == menu_line) { @@ -383,22 +385,21 @@ uint8_t menu_item_toggle_P(const char* str, const char* toggle, menu_func_t func if (toggle == NULL) // print N/A warning message { menu_submenu(func); - return menu_item_ret(); } else // do the actual toggling { lcd_update_enabled = 0; if (func) func(); lcd_update_enabled = 1; - return menu_item_ret(); } + menu_item_ret(); + return; } } menu_item++; - return 0; } -uint8_t menu_item_gcode_P(const char* str, const char* str_gcode) +void menu_item_gcode_P(const char* str, const char* str_gcode) { if (menu_item == menu_line) { @@ -406,11 +407,11 @@ uint8_t menu_item_gcode_P(const char* str, const char* str_gcode) if (menu_clicked && (lcd_encoder == menu_item)) { if (str_gcode) enquecommand_P(str_gcode); - return menu_item_ret(); + menu_item_ret(); + return; } } menu_item++; - return 0; } const char menu_fmt_int3[] PROGMEM = "%c%.15S:%s%3d"; @@ -493,7 +494,7 @@ static void _menu_edit_P(void) } template -uint8_t menu_item_edit_P(const char* str, T pval, int16_t min_val, int16_t max_val) +void menu_item_edit_P(const char* str, T pval, int16_t min_val, int16_t max_val) { menu_data_edit_t* _md = (menu_data_edit_t*)&(menu_data[0]); if (menu_item == menu_line) @@ -511,15 +512,15 @@ uint8_t menu_item_edit_P(const char* str, T pval, int16_t min_val, int16_t max_v _md->minEditValue = min_val; _md->maxEditValue = max_val; lcd_encoder = *pval; - return menu_item_ret(); + menu_item_ret(); + return; } } menu_item++; - return 0; } -template uint8_t menu_item_edit_P(const char* str, int16_t *pval, int16_t min_val, int16_t max_val); -template uint8_t menu_item_edit_P(const char* str, uint8_t *pval, int16_t min_val, int16_t max_val); +template void menu_item_edit_P(const char* str, int16_t *pval, int16_t min_val, int16_t max_val); +template void menu_item_edit_P(const char* str, uint8_t *pval, int16_t min_val, int16_t max_val); static uint8_t progressbar_block_count = 0; static uint16_t progressbar_total = 0; diff --git a/Firmware/menu.h b/Firmware/menu.h index a0a6f845f..0735bcd5e 100755 --- a/Firmware/menu.h +++ b/Firmware/menu.h @@ -82,7 +82,7 @@ extern void menu_back_if_clicked(void); extern void menu_submenu(menu_func_t submenu, const bool feedback=false); extern void menu_submenu_no_reset(menu_func_t submenu, const bool feedback=false); -extern uint8_t menu_item_ret(void); +extern void menu_item_ret(void); //extern int menu_draw_item_printf_P(char type_char, const char* format, ...); @@ -96,33 +96,33 @@ extern void menu_item_dummy(void); extern uint8_t menu_item_text_P(const char* str); #define MENU_ITEM_SUBMENU_P(str, submenu) do { menu_item_submenu_P(str, submenu); } while (0) -extern uint8_t menu_item_submenu_P(const char* str, menu_func_t submenu); +extern void menu_item_submenu_P(const char* str, menu_func_t submenu); #define MENU_ITEM_SUBMENU_E(sheet, submenu) do { menu_item_submenu_E(sheet, submenu); } while (0) -extern uint8_t menu_item_submenu_E(const Sheet &sheet, menu_func_t submenu); +extern void menu_item_submenu_E(const Sheet &sheet, menu_func_t submenu); #define MENU_ITEM_FUNCTION_E(sheet, submenu) do { menu_item_function_E(sheet, submenu); } while (0) -extern uint8_t menu_item_function_E(const Sheet &sheet, menu_func_t func); +extern void menu_item_function_E(const Sheet &sheet, menu_func_t func); #define MENU_ITEM_BACK_P(str) do { menu_item_back_P(str); } while (0) -extern uint8_t menu_item_back_P(const char* str); +extern void menu_item_back_P(const char* str); // leaving menu - this condition must be immediately before MENU_ITEM_BACK_P #define ON_MENU_LEAVE(func) do { if (menu_item_leave()){ func } } while (0) extern bool menu_item_leave(); #define MENU_ITEM_FUNCTION_P(str, func) do { menu_item_function_P(str, func); } while (0) -extern uint8_t menu_item_function_P(const char* str, menu_func_t func); +extern void menu_item_function_P(const char* str, menu_func_t func); #define MENU_ITEM_FUNCTION_NR_P(str, number, func, fn_par) do { menu_item_function_P(str, number, func, fn_par); } while (0) -extern uint8_t menu_item_function_P(const char* str, char number, void (*func)(uint8_t), uint8_t fn_par); +extern void menu_item_function_P(const char* str, char number, void (*func)(uint8_t), uint8_t fn_par); #define MENU_ITEM_TOGGLE_P(str, toggle, func) do { menu_item_toggle_P(str, toggle, func, 0x02); } while (0) #define MENU_ITEM_TOGGLE(str, toggle, func) do { menu_item_toggle_P(str, toggle, func, 0x00); } while (0) -extern uint8_t menu_item_toggle_P(const char* str, const char* toggle, menu_func_t func, const uint8_t settings); +extern void menu_item_toggle_P(const char* str, const char* toggle, menu_func_t func, const uint8_t settings); #define MENU_ITEM_GCODE_P(str, str_gcode) do { menu_item_gcode_P(str, str_gcode); } while (0) -extern uint8_t menu_item_gcode_P(const char* str, const char* str_gcode); +extern void menu_item_gcode_P(const char* str, const char* str_gcode); extern const char menu_fmt_int3[]; @@ -145,7 +145,7 @@ extern void menu_format_sheet_E(const Sheet &sheet_E, SheetFormatBuffer &buffer) #define MENU_ITEM_EDIT_int3_P(str, pval, minval, maxval) do { menu_item_edit_P(str, pval, minval, maxval); } while (0) //#define MENU_ITEM_EDIT_int3_P(str, pval, minval, maxval) MENU_ITEM_EDIT(int3, str, pval, minval, maxval) template -extern uint8_t menu_item_edit_P(const char* str, T pval, int16_t min_val, int16_t max_val); +extern void menu_item_edit_P(const char* str, T pval, int16_t min_val, int16_t max_val); template extern void menu_draw_P(char chr, const char* str, T val); diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index 37eb99bf9..9b3fa9e4a 100644 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -291,7 +291,7 @@ static void lcd_implementation_drawmenu_sddirectory(uint8_t row, const char* lon #define MENU_ITEM_SDFILE(str_fn, str_fnl) do { menu_item_sdfile(str_fn, str_fnl); } while (0) -uint8_t menu_item_sddir(const char* str_fn, char* str_fnl) +static void menu_item_sddir(const char* str_fn, char* str_fnl) { if (menu_item == menu_line) { @@ -304,14 +304,14 @@ uint8_t menu_item_sddir(const char* str_fn, char* str_fnl) lcd_update_enabled = false; menu_action_sddirectory(str_fn); lcd_update_enabled = true; - return menu_item_ret(); + menu_item_ret(); + return; } } menu_item++; - return 0; } -static uint8_t menu_item_sdfile(const char* str_fn, char* str_fnl) +static void menu_item_sdfile(const char* str_fn, char* str_fnl) { if (menu_item == menu_line) { @@ -324,11 +324,11 @@ static uint8_t menu_item_sdfile(const char* str_fn, char* str_fnl) lcd_update_enabled = false; menu_action_sdfile(str_fn); lcd_update_enabled = true; - return menu_item_ret(); + menu_item_ret(); + return; } } menu_item++; - return 0; } // Print temperature (nozzle/bed) (9 chars total) @@ -5454,7 +5454,7 @@ static void lcd_advance_edit_K(void) } } -static uint8_t lcd_advance_K() +static void lcd_advance_K() { if (menu_item == menu_line) { @@ -5467,11 +5467,11 @@ static uint8_t lcd_advance_K() { menu_submenu_no_reset(lcd_advance_edit_K); lcd_encoder = 100. * extruder_advance_K; - return menu_item_ret(); + menu_item_ret(); + return; } } menu_item++; - return 0; } #define MENU_ITEM_EDIT_advance_K() do { lcd_advance_K(); } while (0)