Fix ADC_KEYPAD middle button (#12493)

Marlin/src/inc/Conditionals_LCD.h

@@ -309,6 +309,9 @@
 #define HAS_SHIFT_ENCODER   (!HAS_ADC_BUTTONS && (ENABLED(REPRAPWORLD_KEYPAD) || (HAS_SPI_LCD && DISABLED(NEWPANEL))))
 #define HAS_ENCODER_WHEEL   (!HAS_ADC_BUTTONS && ENABLED(NEWPANEL))
 
+// I2C buttons must be read in the main thread
+#define HAS_SLOW_BUTTONS (ENABLED(LCD_I2C_VIKI) || ENABLED(LCD_I2C_PANELOLU2))
+
 #if HAS_GRAPHICAL_LCD
   /**
    * Default LCD contrast for Graphical LCD displays

Marlin/src/lcd/ultralcd.cpp

@@ -79,13 +79,13 @@
  #include "../feature/bedlevel/bedlevel.h"
 #endif
 
-#if DISABLED(LCD_USE_I2C_BUZZER)
+#if HAS_BUZZER
   #include "../libs/buzzer.h"
 #endif
 
 #if HAS_ENCODER_ACTION
   volatile uint8_t MarlinUI::buttons;
-  #if ENABLED(LCD_HAS_SLOW_BUTTONS)
+  #if HAS_SLOW_BUTTONS
     volatile uint8_t MarlinUI::slow_buttons;
   #endif
 #endif
@@ -248,7 +248,7 @@ void MarlinUI::init() {
     lcd_sd_status = 2; // UNKNOWN
   #endif
 
-  #if HAS_ENCODER_ACTION && ENABLED(LCD_HAS_SLOW_BUTTONS)
+  #if HAS_ENCODER_ACTION && HAS_SLOW_BUTTONS
     slow_buttons = 0;
   #endif
 
@@ -307,16 +307,16 @@ bool MarlinUI::get_blink() {
           refresh(LCDVIEW_REDRAW_NOW);
           if (encoderDirection == -1) { // side effect which signals we are inside a menu
             #if HAS_LCD_MENU
-              if      (RRK(EN_REPRAPWORLD_KEYPAD_DOWN))   encoderPosition += ENCODER_STEPS_PER_MENU_ITEM;
-              else if (RRK(EN_REPRAPWORLD_KEYPAD_UP))     encoderPosition -= ENCODER_STEPS_PER_MENU_ITEM;
-              else if (RRK(EN_REPRAPWORLD_KEYPAD_LEFT))   { MenuItem_back::action(); quick_feedback(); }
-              else if (RRK(EN_REPRAPWORLD_KEYPAD_RIGHT))  { return_to_status(); quick_feedback(); }
+              if      (RRK(EN_KEYPAD_DOWN))   encoderPosition += ENCODER_STEPS_PER_MENU_ITEM;
+              else if (RRK(EN_KEYPAD_UP))     encoderPosition -= ENCODER_STEPS_PER_MENU_ITEM;
+              else if (RRK(EN_KEYPAD_LEFT))   { MenuItem_back::action(); quick_feedback(); }
+              else if (RRK(EN_KEYPAD_RIGHT))  { return_to_status(); quick_feedback(); }
             #endif
           }
-          else if (RRK(EN_REPRAPWORLD_KEYPAD_DOWN))     encoderPosition -= ENCODER_PULSES_PER_STEP;
-          else if (RRK(EN_REPRAPWORLD_KEYPAD_UP))       encoderPosition += ENCODER_PULSES_PER_STEP;
-          else if (RRK(EN_REPRAPWORLD_KEYPAD_LEFT))     { MenuItem_back::action(); quick_feedback(); }
-          else if (RRK(EN_REPRAPWORLD_KEYPAD_RIGHT))    encoderPosition = 0;
+          else if (RRK(EN_KEYPAD_DOWN))     encoderPosition -= ENCODER_PULSES_PER_STEP;
+          else if (RRK(EN_KEYPAD_UP))       encoderPosition += ENCODER_PULSES_PER_STEP;
+          else if (RRK(EN_KEYPAD_LEFT))     { MenuItem_back::action(); quick_feedback(); }
+          else if (RRK(EN_KEYPAD_RIGHT))    encoderPosition = 0;
         #endif
         next_button_update_ms = millis() + ADC_MIN_KEY_DELAY;
         return true;
@@ -326,10 +326,10 @@ bool MarlinUI::get_blink() {
 
       static uint8_t keypad_debounce = 0;
 
-      if (!RRK( EN_REPRAPWORLD_KEYPAD_F1    | EN_REPRAPWORLD_KEYPAD_F2
-              | EN_REPRAPWORLD_KEYPAD_F3    | EN_REPRAPWORLD_KEYPAD_DOWN
-              | EN_REPRAPWORLD_KEYPAD_RIGHT | EN_REPRAPWORLD_KEYPAD_MIDDLE
-              | EN_REPRAPWORLD_KEYPAD_UP    | EN_REPRAPWORLD_KEYPAD_LEFT )
+      if (!RRK( EN_KEYPAD_F1    | EN_KEYPAD_F2
+              | EN_KEYPAD_F3    | EN_KEYPAD_DOWN
+              | EN_KEYPAD_RIGHT | EN_KEYPAD_MIDDLE
+              | EN_KEYPAD_UP    | EN_KEYPAD_LEFT )
       ) {
         if (keypad_debounce > 0) keypad_debounce--;
       }
@@ -340,26 +340,26 @@ bool MarlinUI::get_blink() {
 
         #if HAS_LCD_MENU
 
-          if (RRK(EN_REPRAPWORLD_KEYPAD_MIDDLE))  goto_screen(menu_move);
+          if (RRK(EN_KEYPAD_MIDDLE))  goto_screen(menu_move);
 
           #if DISABLED(DELTA) && Z_HOME_DIR == -1
-            if (RRK(EN_REPRAPWORLD_KEYPAD_F2))    _reprapworld_keypad_move(Z_AXIS,  1);
+            if (RRK(EN_KEYPAD_F2))    _reprapworld_keypad_move(Z_AXIS,  1);
           #endif
 
           if (homed) {
             #if ENABLED(DELTA) || Z_HOME_DIR != -1
-              if (RRK(EN_REPRAPWORLD_KEYPAD_F2))  _reprapworld_keypad_move(Z_AXIS,  1);
+              if (RRK(EN_KEYPAD_F2))  _reprapworld_keypad_move(Z_AXIS,  1);
             #endif
-            if (RRK(EN_REPRAPWORLD_KEYPAD_F3))    _reprapworld_keypad_move(Z_AXIS, -1);
-            if (RRK(EN_REPRAPWORLD_KEYPAD_LEFT))  _reprapworld_keypad_move(X_AXIS, -1);
-            if (RRK(EN_REPRAPWORLD_KEYPAD_RIGHT)) _reprapworld_keypad_move(X_AXIS,  1);
-            if (RRK(EN_REPRAPWORLD_KEYPAD_DOWN))  _reprapworld_keypad_move(Y_AXIS,  1);
-            if (RRK(EN_REPRAPWORLD_KEYPAD_UP))    _reprapworld_keypad_move(Y_AXIS, -1);
+            if (RRK(EN_KEYPAD_F3))    _reprapworld_keypad_move(Z_AXIS, -1);
+            if (RRK(EN_KEYPAD_LEFT))  _reprapworld_keypad_move(X_AXIS, -1);
+            if (RRK(EN_KEYPAD_RIGHT)) _reprapworld_keypad_move(X_AXIS,  1);
+            if (RRK(EN_KEYPAD_DOWN))  _reprapworld_keypad_move(Y_AXIS,  1);
+            if (RRK(EN_KEYPAD_UP))    _reprapworld_keypad_move(Y_AXIS, -1);
           }
 
         #endif // HAS_LCD_MENU
 
-        if (!homed && RRK(EN_REPRAPWORLD_KEYPAD_F1)) enqueue_and_echo_commands_P(PSTR("G28"));
+        if (!homed && RRK(EN_KEYPAD_F1)) enqueue_and_echo_commands_P(PSTR("G28"));
         return true;
       }
 
@@ -666,7 +666,7 @@ void MarlinUI::update() {
     }
     else wait_for_unclick = false;
 
-    #if BUTTON_EXISTS(BACK)
+    #if HAS_DIGITAL_BUTTONS && BUTTON_EXISTS(BACK)
       if (LCD_BACK_CLICKED()) {
         quick_feedback();
         goto_previous_screen();
@@ -717,7 +717,7 @@ void MarlinUI::update() {
 
     #if HAS_ENCODER_ACTION
 
-      #if ENABLED(LCD_HAS_SLOW_BUTTONS)
+      #if HAS_SLOW_BUTTONS
         slow_buttons = read_slow_buttons(); // Buttons that take too long to read in interrupt context
       #endif
 
@@ -905,14 +905,14 @@ void MarlinUI::update() {
 
   static const _stADCKeypadTable_ stADCKeyTable[] PROGMEM = {
     // VALUE_MIN, VALUE_MAX, KEY
-    { 4000, 4096, 1 + BLEN_REPRAPWORLD_KEYPAD_F1     }, // F1
-    { 4000, 4096, 1 + BLEN_REPRAPWORLD_KEYPAD_F2     }, // F2
-    { 4000, 4096, 1 + BLEN_REPRAPWORLD_KEYPAD_F3     }, // F3
-    {  300,  500, 1 + BLEN_REPRAPWORLD_KEYPAD_LEFT   }, // LEFT
-    { 1900, 2200, 1 + BLEN_REPRAPWORLD_KEYPAD_RIGHT  }, // RIGHT
-    {  570,  870, 1 + BLEN_REPRAPWORLD_KEYPAD_UP     }, // UP
-    { 2670, 2870, 1 + BLEN_REPRAPWORLD_KEYPAD_DOWN   }, // DOWN
-    { 1150, 1450, 1 + BLEN_REPRAPWORLD_KEYPAD_MIDDLE }, // ENTER
+    { 4000, 4096, 1 + BLEN_KEYPAD_F1     }, // F1
+    { 4000, 4096, 1 + BLEN_KEYPAD_F2     }, // F2
+    { 4000, 4096, 1 + BLEN_KEYPAD_F3     }, // F3
+    {  300,  500, 1 + BLEN_KEYPAD_LEFT   }, // LEFT
+    { 1900, 2200, 1 + BLEN_KEYPAD_RIGHT  }, // RIGHT
+    {  570,  870, 1 + BLEN_KEYPAD_UP     }, // UP
+    { 2670, 2870, 1 + BLEN_KEYPAD_DOWN   }, // DOWN
+    { 1150, 1450, 1 + BLEN_KEYPAD_MIDDLE }, // ENTER
   };
 
   uint8_t get_ADC_keyValue(void) {
@@ -1025,7 +1025,7 @@ void MarlinUI::update() {
         #endif // UP || DWN || LFT || RT
 
         buttons = newbutton
-          #if ENABLED(LCD_HAS_SLOW_BUTTONS)
+          #if HAS_SLOW_BUTTONS
             | slow_buttons
           #endif
         ;
@@ -1087,7 +1087,7 @@ void MarlinUI::update() {
     #endif // HAS_ENCODER_WHEEL
   }
 
-  #if ENABLED(LCD_HAS_SLOW_BUTTONS)
+  #if HAS_SLOW_BUTTONS
 
     uint8_t MarlinUI::read_slow_buttons() {
       #if ENABLED(LCD_I2C_TYPE_MCP23017)
@@ -1102,7 +1102,7 @@ void MarlinUI::update() {
       #endif // LCD_I2C_TYPE_MCP23017
     }
 
-  #endif // LCD_HAS_SLOW_BUTTONS
+  #endif
 
 #endif // HAS_ENCODER_ACTION
 

Marlin/src/lcd/ultralcd.h

@@ -88,28 +88,36 @@
 
 #endif
 
+// REPRAPWORLD_KEYPAD (and ADC_KEYPAD)
 #if ENABLED(REPRAPWORLD_KEYPAD)
-  #define REPRAPWORLD_BTN_OFFSET          0 // Bit offset into buttons for shift register values
-
-  #define BLEN_REPRAPWORLD_KEYPAD_F3      0
-  #define BLEN_REPRAPWORLD_KEYPAD_F2      1
-  #define BLEN_REPRAPWORLD_KEYPAD_F1      2
-  #define BLEN_REPRAPWORLD_KEYPAD_DOWN    3
-  #define BLEN_REPRAPWORLD_KEYPAD_RIGHT   4
-  #define BLEN_REPRAPWORLD_KEYPAD_MIDDLE  5
-  #define BLEN_REPRAPWORLD_KEYPAD_UP      6
-  #define BLEN_REPRAPWORLD_KEYPAD_LEFT    7
-
-  #define EN_REPRAPWORLD_KEYPAD_F1        (_BV(REPRAPWORLD_BTN_OFFSET + BLEN_REPRAPWORLD_KEYPAD_F1))
-  #define EN_REPRAPWORLD_KEYPAD_F2        (_BV(REPRAPWORLD_BTN_OFFSET + BLEN_REPRAPWORLD_KEYPAD_F2))
-  #define EN_REPRAPWORLD_KEYPAD_F3        (_BV(REPRAPWORLD_BTN_OFFSET + BLEN_REPRAPWORLD_KEYPAD_F3))
-  #define EN_REPRAPWORLD_KEYPAD_DOWN      (_BV(REPRAPWORLD_BTN_OFFSET + BLEN_REPRAPWORLD_KEYPAD_DOWN))
-  #define EN_REPRAPWORLD_KEYPAD_RIGHT     (_BV(REPRAPWORLD_BTN_OFFSET + BLEN_REPRAPWORLD_KEYPAD_RIGHT))
-  #define EN_REPRAPWORLD_KEYPAD_MIDDLE    (_BV(REPRAPWORLD_BTN_OFFSET + BLEN_REPRAPWORLD_KEYPAD_MIDDLE))
-  #define EN_REPRAPWORLD_KEYPAD_UP        (_BV(REPRAPWORLD_BTN_OFFSET + BLEN_REPRAPWORLD_KEYPAD_UP))
-  #define EN_REPRAPWORLD_KEYPAD_LEFT      (_BV(REPRAPWORLD_BTN_OFFSET + BLEN_REPRAPWORLD_KEYPAD_LEFT))
+  #define BTN_OFFSET          0 // Bit offset into buttons for shift register values
+
+  #define BLEN_KEYPAD_F3      0
+  #define BLEN_KEYPAD_F2      1
+  #define BLEN_KEYPAD_F1      2
+  #define BLEN_KEYPAD_DOWN    3
+  #define BLEN_KEYPAD_RIGHT   4
+  #define BLEN_KEYPAD_MIDDLE  5
+  #define BLEN_KEYPAD_UP      6
+  #define BLEN_KEYPAD_LEFT    7
+
+  #define EN_KEYPAD_F1      _BV(BTN_OFFSET + BLEN_KEYPAD_F1)
+  #define EN_KEYPAD_F2      _BV(BTN_OFFSET + BLEN_KEYPAD_F2)
+  #define EN_KEYPAD_F3      _BV(BTN_OFFSET + BLEN_KEYPAD_F3)
+  #define EN_KEYPAD_DOWN    _BV(BTN_OFFSET + BLEN_KEYPAD_DOWN)
+  #define EN_KEYPAD_RIGHT   _BV(BTN_OFFSET + BLEN_KEYPAD_RIGHT)
+  #define EN_KEYPAD_MIDDLE  _BV(BTN_OFFSET + BLEN_KEYPAD_MIDDLE)
+  #define EN_KEYPAD_UP      _BV(BTN_OFFSET + BLEN_KEYPAD_UP)
+  #define EN_KEYPAD_LEFT    _BV(BTN_OFFSET + BLEN_KEYPAD_LEFT)
 
   #define RRK(B) (keypad_buttons & (B))
+
+  #ifdef EN_C
+    #define BUTTON_CLICK() ((buttons & EN_C) || RRK(EN_KEYPAD_MIDDLE))
+  #else
+    #define BUTTON_CLICK() RRK(EN_KEYPAD_MIDDLE)
+  #endif
+
 #endif
 
 #if HAS_DIGITAL_BUTTONS
@@ -129,26 +137,12 @@
     #define EN_C _BV(BLEN_C)
   #endif
 
-  #if BUTTON_EXISTS(BACK)
-    #define BLEN_D 3
-    #define EN_D _BV(BLEN_D)
-    #define LCD_BACK_CLICKED() (buttons & EN_D)
-  #endif
-
-  #if ENABLED(REPRAPWORLD_KEYPAD)
-
-    #ifdef EN_C
-      #define BUTTON_CLICK() ((buttons & EN_C) || RRK(EN_REPRAPWORLD_KEYPAD_MIDDLE))
-    #else
-      #define BUTTON_CLICK() RRK(EN_REPRAPWORLD_KEYPAD_MIDDLE)
-    #endif
-
-  #elif ENABLED(LCD_I2C_VIKI)
+  #if ENABLED(LCD_I2C_VIKI)
 
     #define B_I2C_BTN_OFFSET 3 // (the first three bit positions reserved for EN_A, EN_B, EN_C)
 
     // button and encoder bit positions within 'buttons'
-    #define B_LE (BUTTON_LEFT   << B_I2C_BTN_OFFSET)    // The remaining normalized buttons are all read via I2C
+    #define B_LE (BUTTON_LEFT   << B_I2C_BTN_OFFSET)      // The remaining normalized buttons are all read via I2C
     #define B_UP (BUTTON_UP     << B_I2C_BTN_OFFSET)
     #define B_MI (BUTTON_SELECT << B_I2C_BTN_OFFSET)
     #define B_DW (BUTTON_DOWN   << B_I2C_BTN_OFFSET)
@@ -156,13 +150,12 @@
 
     #if BUTTON_EXISTS(ENC)                                // The pause/stop/restart button is connected to BTN_ENC when used
       #define B_ST (EN_C)                                 // Map the pause/stop/resume button into its normalized functional name
-      #define BUTTON_CLICK() (buttons & (B_MI|B_RI|B_ST))  // Pause/stop also acts as click until a proper pause/stop is implemented.
+      #define BUTTON_CLICK() (buttons & (B_MI|B_RI|B_ST)) // Pause/stop also acts as click until a proper pause/stop is implemented.
     #else
       #define BUTTON_CLICK() (buttons & (B_MI|B_RI))
     #endif
 
     // I2C buttons take too long to read inside an interrupt context and so we read them during lcd_update
-    #define LCD_HAS_SLOW_BUTTONS
 
   #elif ENABLED(LCD_I2C_PANELOLU2)
 
@@ -174,16 +167,13 @@
 
       #define BUTTON_CLICK() (buttons & B_MI)
 
-      // I2C buttons take too long to read inside an interrupt context and so we read them during lcd_update
-      #define LCD_HAS_SLOW_BUTTONS
-
     #endif
 
   #endif
 
 #else
 
-  #define BUTTON_EXISTS(BN) 0
+  #define BUTTON_EXISTS(BN) false
 
   // Shift register bits correspond to buttons:
   #define BL_LE 7   // Left
@@ -202,6 +192,14 @@
 
 #endif
 
+#if BUTTON_EXISTS(BACK)
+  #define BLEN_D 3
+  #define EN_D _BV(BLEN_D)
+  #define LCD_BACK_CLICKED() (buttons & EN_D)
+#else
+  #define LCD_BACK_CLICKED() false
+#endif
+
 #ifndef BUTTON_CLICK
   #ifdef EN_C
     #define BUTTON_CLICK() (buttons & EN_C)
@@ -471,7 +469,7 @@ public:
       static volatile uint8_t keypad_buttons;
       static bool handle_keypad();
     #endif
-    #if ENABLED(LCD_HAS_SLOW_BUTTONS)
+    #if HAS_SLOW_BUTTONS
       static volatile uint8_t slow_buttons;
       static uint8_t read_slow_buttons();
     #endif