From bb5ac93921857888c0115c866a596d87159440c2 Mon Sep 17 00:00:00 2001 From: Marek Bel Date: Wed, 22 Aug 2018 18:55:42 +0200 Subject: [PATCH 1/3] Add missing compile time check for ShortTimer fitting into menu_data. Convert preprocessor checks to shorter and more accurate static_assert. Remove redundant macro MENU_DATA_EDIT_SIZE. Decrease scope of menu_stack and asociated macro. No functional change. --- Firmware/menu.cpp | 14 ++++---------- Firmware/menu.h | 11 ++++++++--- Firmware/ultralcd.cpp | 23 +++++++---------------- 3 files changed, 19 insertions(+), 29 deletions(-) diff --git a/Firmware/menu.cpp b/Firmware/menu.cpp index b63629ed..66e2e1c3 100644 --- a/Firmware/menu.cpp +++ b/Firmware/menu.cpp @@ -14,8 +14,9 @@ extern int32_t lcd_encoder; +#define MENU_DEPTH_MAX 4 -menu_record_t menu_stack[MENU_DEPTH_MAX]; +static menu_record_t menu_stack[MENU_DEPTH_MAX]; uint8_t menu_data[MENU_DATA_SIZE]; @@ -33,6 +34,8 @@ uint8_t menu_leaving = 0; menu_func_t menu_menu = 0; +static_assert(sizeof(menu_data)>= sizeof(menu_data_edit_t),"menu_data_edit_t doesn't fit into menu_data"); + void menu_goto(menu_func_t menu, const uint32_t encoder, const bool feedback, bool reset_menu_state) { @@ -296,15 +299,6 @@ void menu_draw_float13(char chr, const char* str, float val) lcd_printf_P(menu_fmt_float13, chr, str, spaces, val); } -typedef struct -{ - //Variables used when editing values. - const char* editLabel; - void* editValue; - int32_t minEditValue; - int32_t maxEditValue; -} menu_data_edit_t; - void _menu_edit_int3(void) { menu_data_edit_t* _md = (menu_data_edit_t*)&(menu_data[0]); diff --git a/Firmware/menu.h b/Firmware/menu.h index 06c90103..af6f3896 100644 --- a/Firmware/menu.h +++ b/Firmware/menu.h @@ -4,9 +4,7 @@ #include -#define MENU_DEPTH_MAX 4 #define MENU_DATA_SIZE 32 -#define MENU_DATA_EDIT_SIZE 12 //Function pointer to menu functions. typedef void (*menu_func_t)(void); @@ -17,7 +15,14 @@ typedef struct int8_t position; } menu_record_t; -extern menu_record_t menu_stack[MENU_DEPTH_MAX]; +typedef struct +{ + //Variables used when editing values. + const char* editLabel; + void* editValue; + int32_t minEditValue; + int32_t maxEditValue; +} menu_data_edit_t; extern uint8_t menu_data[MENU_DATA_SIZE]; diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index 9988815e..1e220849 100644 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -2086,9 +2086,7 @@ static void lcd_support_menu() uint8_t ip[4]; // 4bytes char ip_str[3*4+3+1]; // 16bytes } _menu_data_t; -#if (22 > MENU_DATA_SIZE) -#error "check MENU_DATA_SIZE definition!" -#endif + static_assert(sizeof(menu_data)>= sizeof(_menu_data_t),"_menu_data_t doesn't fit into menu_data"); _menu_data_t* _md = (_menu_data_t*)&(menu_data[0]); if (_md->status == 0 || lcd_draw_update == 2) { @@ -2417,6 +2415,7 @@ static void lcd_menu_AutoLoadFilament() } else { + static_assert(sizeof(menu_data)>=sizeof(ShortTimer), "ShortTimer doesn't fit into menu_data"); ShortTimer* ptimer = (ShortTimer*)&(menu_data[0]); if (!ptimer->running()) ptimer->start(); lcd_set_cursor(0, 0); @@ -2533,9 +2532,7 @@ static void _lcd_move(const char *name, int axis, int min, int max) bool initialized; // 1byte bool endstopsEnabledPrevious; // 1byte } _menu_data_t; -#if (2 > MENU_DATA_SIZE) -#error "check MENU_DATA_SIZE definition!" -#endif + static_assert(sizeof(menu_data)>= sizeof(_menu_data_t),"_menu_data_t doesn't fit into menu_data"); _menu_data_t* _md = (_menu_data_t*)&(menu_data[0]); if (!_md->initialized) { @@ -2739,9 +2736,7 @@ static void _lcd_babystep(int axis, const char *msg) int babystepMem[3]; // 6bytes float babystepMemMM[3]; // 12bytes } _menu_data_t; -#if (19 > MENU_DATA_SIZE) -#error "check MENU_DATA_SIZE definition!" -#endif + static_assert(sizeof(menu_data)>= sizeof(_menu_data_t),"_menu_data_t doesn't fit into menu_data"); _menu_data_t* _md = (_menu_data_t*)&(menu_data[0]); if (_md->status == 0) { @@ -2822,16 +2817,14 @@ static void lcd_babystep_z() typedef struct { // 12bytes + 9bytes = 21bytes total - uint8_t reserved[MENU_DATA_EDIT_SIZE]; //12 bytes reserved for number editing functions + menu_data_edit_t reserved; //12 bytes reserved for number editing functions int8_t status; // 1byte int16_t left; // 2byte int16_t right; // 2byte int16_t front; // 2byte int16_t rear; // 2byte } _menu_data_adjust_bed_t; -#if (21 > MENU_DATA_SIZE) -#error "check MENU_DATA_SIZE definition!" -#endif +static_assert(sizeof(menu_data)>= sizeof(_menu_data_adjust_bed_t),"_menu_data_adjust_bed_t doesn't fit into menu_data"); void lcd_adjust_bed_reset(void) { @@ -5674,9 +5667,7 @@ static void lcd_tune_menu() // it needs to be applied. int16_t extrudemultiply; // 2byte } _menu_data_t; -#if (3 > MENU_DATA_SIZE) -#error "check MENU_DATA_SIZE definition!" -#endif + static_assert(sizeof(menu_data)>= sizeof(_menu_data_t),"_menu_data_t doesn't fit into menu_data"); _menu_data_t* _md = (_menu_data_t*)&(menu_data[0]); if (_md->status == 0) { From 07d07831f1ba99a2ce07176dfd9c1f427136996d Mon Sep 17 00:00:00 2001 From: Marek Bel Date: Wed, 22 Aug 2018 19:40:12 +0200 Subject: [PATCH 2/3] Add portability note. --- Firmware/menu.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Firmware/menu.cpp b/Firmware/menu.cpp index 66e2e1c3..7fa17746 100644 --- a/Firmware/menu.cpp +++ b/Firmware/menu.cpp @@ -19,6 +19,9 @@ extern int32_t lcd_encoder; static menu_record_t menu_stack[MENU_DEPTH_MAX]; uint8_t menu_data[MENU_DATA_SIZE]; +#ifndef __AVR__ +#error "menu_data is non-portable to non 8bit processor" +#endif uint8_t menu_depth = 0; From 3780516f74e9b8837d1ec93919f5a8554750e6bc Mon Sep 17 00:00:00 2001 From: Marek Bel Date: Wed, 22 Aug 2018 19:47:29 +0200 Subject: [PATCH 3/3] PFW-512 Reserve space for MENU_ITEM_EDIT_int3_P in shared memory in lcd_tune_menu(). --- Firmware/ultralcd.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index 1e220849..fb4a083c 100644 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -5660,12 +5660,12 @@ static void lcd_colorprint_change() { static void lcd_tune_menu() { typedef struct - { // 3bytes total - // To recognize, whether the menu has been just initialized. - int8_t status; // 1byte - // Backup of extrudemultiply, to recognize, that the value has been changed and - // it needs to be applied. - int16_t extrudemultiply; // 2byte + { + menu_data_edit_t reserved; //!< reserved for number editing functions + int8_t status; //!< To recognize, whether the menu has been just initialized. + //! Backup of extrudemultiply, to recognize, that the value has been changed and + //! it needs to be applied. + int16_t extrudemultiply; } _menu_data_t; static_assert(sizeof(menu_data)>= sizeof(_menu_data_t),"_menu_data_t doesn't fit into menu_data"); _menu_data_t* _md = (_menu_data_t*)&(menu_data[0]);