From 90a10a692a7715e3ccc99a9cb027ce7532fd8738 Mon Sep 17 00:00:00 2001 From: Marek Bel Date: Tue, 18 Sep 2018 14:00:57 +0200 Subject: [PATCH 1/5] Enable button debouncing also in modal mode (!lcd_update_enabled). In lcd_clicked() consume click event immediately. --- Firmware/lcd.cpp | 89 ++++++++++++++++++++----------------------- Firmware/menu.cpp | 2 + Firmware/ultralcd.cpp | 2 + 3 files changed, 46 insertions(+), 47 deletions(-) diff --git a/Firmware/lcd.cpp b/Firmware/lcd.cpp index 5ac6c317..7cd67fe3 100644 --- a/Firmware/lcd.cpp +++ b/Firmware/lcd.cpp @@ -686,7 +686,11 @@ LongTimer lcd_timeoutToStatus; uint8_t lcd_clicked(void) { bool clicked = LCD_CLICKED; - if(clicked) lcd_button_pressed = 0; + if(clicked) + { + lcd_button_pressed = 0; + lcd_buttons &= 0xff^EN_C; + } return clicked; } @@ -709,7 +713,7 @@ Sound_MakeSound(e_SOUND_TYPE_ButtonEcho); void lcd_quick_feedback(void) { lcd_draw_update = 2; - lcd_button_pressed = false; + lcd_button_pressed = false; lcd_beeper_quick_feedback(); } @@ -766,51 +770,42 @@ void lcd_buttons_update(void) uint8_t newbutton = 0; if (READ(BTN_EN1) == 0) newbutton |= EN_A; if (READ(BTN_EN2) == 0) newbutton |= EN_B; - if (lcd_update_enabled) - { //if we are in non-modal mode, long press can be used and short press triggers with button release - if (READ(BTN_ENC) == 0) - { //button is pressed - lcd_timeoutToStatus.start(); - if (!buttonBlanking.running() || buttonBlanking.expired(BUTTON_BLANKING_TIME)) { - buttonBlanking.start(); - safetyTimer.start(); - if ((lcd_button_pressed == 0) && (lcd_long_press_active == 0)) - { - longPressTimer.start(); - lcd_button_pressed = 1; - } - else - { - if (longPressTimer.expired(LONG_PRESS_TIME)) - { - lcd_long_press_active = 1; - if (lcd_longpress_func) - lcd_longpress_func(); - } - } - } - } - else - { //button not pressed - if (lcd_button_pressed) - { //button was released - buttonBlanking.start(); - if (lcd_long_press_active == 0) - { //button released before long press gets activated - newbutton |= EN_C; - } - //else if (menu_menu == lcd_move_z) lcd_quick_feedback(); - //lcd_button_pressed is set back to false via lcd_quick_feedback function - } - else - lcd_long_press_active = 0; - } - } - else - { //we are in modal mode - if (READ(BTN_ENC) == 0) - newbutton |= EN_C; - } + + if (READ(BTN_ENC) == 0) + { //button is pressed + lcd_timeoutToStatus.start(); + if (!buttonBlanking.running() || buttonBlanking.expired(BUTTON_BLANKING_TIME)) { + buttonBlanking.start(); + safetyTimer.start(); + if ((lcd_button_pressed == 0) && (lcd_long_press_active == 0)) + { + //long press is not possible in modal mode + if (lcd_update_enabled) longPressTimer.start(); + lcd_button_pressed = 1; + } + else if (longPressTimer.expired(LONG_PRESS_TIME)) + { + lcd_long_press_active = 1; + if (lcd_longpress_func) + lcd_longpress_func(); + } + } + } + else + { //button not pressed + if (lcd_button_pressed) + { //button was released + buttonBlanking.start(); + if (lcd_long_press_active == 0) + { //button released before long press gets activated + newbutton |= EN_C; + } + //else if (menu_menu == lcd_move_z) lcd_quick_feedback(); + //lcd_button_pressed is set back to false via lcd_quick_feedback function + } + else + lcd_long_press_active = 0; + } lcd_buttons = newbutton; //manage encoder rotation diff --git a/Firmware/menu.cpp b/Firmware/menu.cpp index bd523826..a92fc857 100644 --- a/Firmware/menu.cpp +++ b/Firmware/menu.cpp @@ -236,6 +236,8 @@ uint8_t menu_item_function_P(const char* str, menu_func_t func) if (menu_clicked && (lcd_encoder == menu_item)) { menu_clicked = false; + lcd_button_pressed = 0; + lcd_buttons &= 0xff^EN_C; lcd_update_enabled = 0; if (func) func(); lcd_update_enabled = 1; diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index cd73912e..89e6c6c2 100644 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -463,6 +463,8 @@ static uint8_t menu_item_sdfile(const char* } if (menu_clicked && (lcd_encoder == menu_item)) { + lcd_button_pressed = 0; + lcd_buttons &= 0xff^EN_C; menu_action_sdfile(str_fn); return menu_item_ret(); } From f532da4b17654c1563f9d03377158724bcc2f5be Mon Sep 17 00:00:00 2001 From: Marek Bel Date: Mon, 17 Sep 2018 21:36:04 +0200 Subject: [PATCH 2/5] Remove duplicate button debouncing code. Save 254B flash. --- Firmware/ultralcd.cpp | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index 89e6c6c2..edadda67 100644 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -3253,9 +3253,6 @@ void lcd_show_fullscreen_message_and_wait_P(const char *msg) for (uint8_t i = 0; i < 100; ++ i) { delay_keep_alive(50); if (lcd_clicked()) { - while (lcd_clicked()) ; - delay(10); - while (lcd_clicked()) ; if (msg_next == NULL) { KEEPALIVE_STATE(IN_HANDLER); lcd_set_custom_characters(); @@ -3289,9 +3286,6 @@ void lcd_wait_for_click() manage_heater(); manage_inactivity(true); if (lcd_clicked()) { - while (lcd_clicked()) ; - delay(10); - while (lcd_clicked()) ; KEEPALIVE_STATE(IN_HANDLER); return; } @@ -3338,9 +3332,6 @@ int8_t lcd_show_multiscreen_message_yes_no_and_wait_P(const char *msg, bool allo } } if (lcd_clicked()) { - while (lcd_clicked()); - delay(10); - while (lcd_clicked()); if (msg_next == NULL) { //KEEPALIVE_STATE(IN_HANDLER); lcd_set_custom_characters(); @@ -3415,9 +3406,6 @@ int8_t lcd_show_fullscreen_message_yes_no_and_wait_P(const char *msg, bool allow enc_dif = lcd_encoder_diff; } if (lcd_clicked()) { - while (lcd_clicked()); - delay(10); - while (lcd_clicked()); KEEPALIVE_STATE(IN_HANDLER); return yes; } @@ -3534,9 +3522,6 @@ void lcd_diag_show_end_stops() manage_inactivity(true); lcd_show_end_stops(); if (lcd_clicked()) { - while (lcd_clicked()) ; - delay(10); - while (lcd_clicked()) ; break; } } @@ -4299,9 +4284,6 @@ void lcd_v2_calibration() for (int i = 0; i < 20; i++) { //wait max. 2s delay_keep_alive(100); if (lcd_clicked()) { - while (lcd_clicked()); - delay(10); - while (lcd_clicked()); break; } } @@ -4901,9 +4883,6 @@ void bowden_menu() { } if (lcd_clicked()) { - while (lcd_clicked()); - delay(10); - while (lcd_clicked()); lcd_clear(); while (1) { @@ -4935,9 +4914,6 @@ void bowden_menu() { } delay(100); if (lcd_clicked()) { - while (lcd_clicked()); - delay(10); - while (lcd_clicked()); EEPROM_save_B(EEPROM_BOWDEN_LENGTH + cursor_pos * 2, &bowden_length[cursor_pos]); if (lcd_show_fullscreen_message_yes_no_and_wait_P(PSTR("Continue with another bowden?"))) { lcd_update_enable(true); @@ -4998,9 +4974,6 @@ static char snmm_stop_print_menu() { //menu for choosing which filaments will be } } if (lcd_clicked()) { - while (lcd_clicked()); - delay(10); - while (lcd_clicked()); KEEPALIVE_STATE(IN_HANDLER); return(cursor_pos - 1); } @@ -5187,9 +5160,6 @@ char reset_menu() { } if (lcd_clicked()) { - while (lcd_clicked()); - delay(10); - while (lcd_clicked()); return(cursor_pos + first); } @@ -5446,9 +5416,6 @@ unsigned char lcd_choose_color() { } if (lcd_clicked()) { - while (lcd_clicked()); - delay(10); - while (lcd_clicked()); switch(cursor_pos + first - 1) { case 0: return 1; break; case 1: return 0; break; From 6ee97468ee461046dd13090c27fb23077365fcaf Mon Sep 17 00:00:00 2001 From: Marek Bel Date: Tue, 18 Sep 2018 10:17:09 +0200 Subject: [PATCH 3/5] Remove lcd_buttons_update() call from all other places than interrupt. There is no known reason, why lcd_buttons_update() should be called from multiple places and multiple contexts. Remove mutex, which is not needed anymore, and wasn't implemented properly anyway (Operation was not atomic.). --- Firmware/lcd.cpp | 5 ----- Firmware/ultralcd.cpp | 1 - 2 files changed, 6 deletions(-) diff --git a/Firmware/lcd.cpp b/Firmware/lcd.cpp index 7cd67fe3..31c7183e 100644 --- a/Firmware/lcd.cpp +++ b/Firmware/lcd.cpp @@ -730,7 +730,6 @@ void lcd_update(uint8_t lcdDrawUpdateOverride) lcd_draw_update = lcdDrawUpdateOverride; if (!lcd_update_enabled) return; - lcd_buttons_update(); if (lcd_lcdupdate_func) lcd_lcdupdate_func(); } @@ -764,9 +763,6 @@ void lcd_update_enable(uint8_t enabled) extern LongTimer safetyTimer; void lcd_buttons_update(void) { - static bool _lock = false; - if (_lock) return; - _lock = true; uint8_t newbutton = 0; if (READ(BTN_EN1) == 0) newbutton |= EN_A; if (READ(BTN_EN2) == 0) newbutton |= EN_B; @@ -843,7 +839,6 @@ void lcd_buttons_update(void) } } lcd_encoder_bits = enc; - _lock = false; } diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index edadda67..35426147 100644 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -7106,7 +7106,6 @@ void ultralcd_init() WRITE(SDCARDDETECT, HIGH); lcd_oldcardstatus = IS_SD_INSERTED; #endif//(SDCARDDETECT > 0) - lcd_buttons_update(); lcd_encoder_diff = 0; } From a7fdfdd2580b8f2efe113a3337513138ae2ad2cd Mon Sep 17 00:00:00 2001 From: Marek Bel Date: Tue, 18 Sep 2018 17:15:13 +0200 Subject: [PATCH 4/5] Put repetitive code into separate function. No change in functionality. --- Firmware/lcd.cpp | 9 +++++++-- Firmware/lcd.h | 7 ++++++- Firmware/menu.cpp | 3 +-- Firmware/ultralcd.cpp | 3 +-- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/Firmware/lcd.cpp b/Firmware/lcd.cpp index 31c7183e..c434aa08 100644 --- a/Firmware/lcd.cpp +++ b/Firmware/lcd.cpp @@ -683,13 +683,18 @@ ShortTimer longPressTimer; LongTimer lcd_timeoutToStatus; +//! @brief Was button clicked? +//! +//! Consume click event, following call would return 0. +//! +//! @retval 0 not clicked +//! @retval nonzero clicked uint8_t lcd_clicked(void) { bool clicked = LCD_CLICKED; if(clicked) { - lcd_button_pressed = 0; - lcd_buttons &= 0xff^EN_C; + lcd_consume_click(); } return clicked; } diff --git a/Firmware/lcd.h b/Firmware/lcd.h index 251344de..8d774b30 100644 --- a/Firmware/lcd.h +++ b/Firmware/lcd.h @@ -131,7 +131,6 @@ extern lcd_lcdupdate_func_t lcd_lcdupdate_func; extern uint8_t lcd_clicked(void); - extern void lcd_beeper_quick_feedback(void); //Cause an LCD refresh, and give the user visual or audible feedback that something has happened @@ -221,6 +220,12 @@ extern void lcd_set_custom_characters_progress(void); extern void lcd_set_custom_characters_nextpage(void); extern void lcd_set_custom_characters_degree(void); +//! @brief Consume click event +inline void lcd_consume_click() +{ + lcd_button_pressed = 0; + lcd_buttons &= 0xff^EN_C; +} #endif //_LCD_H diff --git a/Firmware/menu.cpp b/Firmware/menu.cpp index a92fc857..b002ca0e 100644 --- a/Firmware/menu.cpp +++ b/Firmware/menu.cpp @@ -236,8 +236,7 @@ uint8_t menu_item_function_P(const char* str, menu_func_t func) if (menu_clicked && (lcd_encoder == menu_item)) { menu_clicked = false; - lcd_button_pressed = 0; - lcd_buttons &= 0xff^EN_C; + lcd_consume_click(); lcd_update_enabled = 0; if (func) func(); lcd_update_enabled = 1; diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index 35426147..7b201f2a 100644 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -463,8 +463,7 @@ static uint8_t menu_item_sdfile(const char* } if (menu_clicked && (lcd_encoder == menu_item)) { - lcd_button_pressed = 0; - lcd_buttons &= 0xff^EN_C; + lcd_consume_click(); menu_action_sdfile(str_fn); return menu_item_ret(); } From 05d3b7032ddf156a8bda0d7de9e063314f1adce3 Mon Sep 17 00:00:00 2001 From: Marek Bel Date: Tue, 18 Sep 2018 17:48:11 +0200 Subject: [PATCH 5/5] Update documentation. --- Firmware/lcd.cpp | 3 +++ Firmware/lcd.h | 13 +++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Firmware/lcd.cpp b/Firmware/lcd.cpp index c434aa08..7e8b3ed4 100644 --- a/Firmware/lcd.cpp +++ b/Firmware/lcd.cpp @@ -686,6 +686,9 @@ LongTimer lcd_timeoutToStatus; //! @brief Was button clicked? //! //! Consume click event, following call would return 0. +//! See #LCD_CLICKED macro for version not consuming the event. +//! +//! Generally is used in modal dialogs. //! //! @retval 0 not clicked //! @retval nonzero clicked diff --git a/Firmware/lcd.h b/Firmware/lcd.h index 8d774b30..fe48b339 100644 --- a/Firmware/lcd.h +++ b/Firmware/lcd.h @@ -1,4 +1,4 @@ -//lcd.h +//! @file #ifndef _LCD_H #define _LCD_H @@ -186,7 +186,16 @@ extern void lcd_buttons_update(void); #define EN_A (1<