Use single memcmp to determine if matrix changed. (#13064)

* Use memcmp to determine if matrix changed.

* Firmware size issues.

* Add documentation for the lack of need of MATRIX_ROW_PINS/MATRIX_COL_PINS, when overriding low-level matrix functions.

docs/config_options.md

@@ -51,8 +51,10 @@ This is a C header file that is one of the first things included, and will persi
   * the number of columns in your keyboard's matrix
 * `#define MATRIX_ROW_PINS { D0, D5, B5, B6 }`
   * pins of the rows, from top to bottom
+  * may be omitted by the keyboard designer if matrix reads are handled in an alternate manner. See [low-level matrix overrides](custom_quantum_functions.md?id=low-level-matrix-overrides) for more information.
 * `#define MATRIX_COL_PINS { F1, F0, B0, C7, F4, F5, F6, F7, D4, D6, B4, D7 }`
   * pins of the columns, from left to right
+  * may be omitted by the keyboard designer if matrix reads are handled in an alternate manner. See [low-level matrix overrides](custom_quantum_functions.md?id=low-level-matrix-overrides) for more information.
 * `#define MATRIX_IO_DELAY 30`
   * the delay in microseconds when between changing matrix pin state and reading values
 * `#define UNUSED_PINS { D1, D2, D3, B1, B2, B3 }`
@@ -280,6 +282,7 @@ There are a few different ways to set handedness for split keyboards (listed in
 * `#define MATRIX_ROW_PINS_RIGHT { <row pins> }`
 * `#define MATRIX_COL_PINS_RIGHT { <col pins> }`
   * If you want to specify a different pinout for the right half than the left half, you can define `MATRIX_ROW_PINS_RIGHT`/`MATRIX_COL_PINS_RIGHT`. Currently, the size of `MATRIX_ROW_PINS` must be the same as `MATRIX_ROW_PINS_RIGHT` and likewise for the definition of columns.
+  * may be omitted by the keyboard designer if matrix reads are handled in an alternate manner. See [low-level matrix overrides](custom_quantum_functions.md?id=low-level-matrix-overrides) for more information.
 
 * `#define DIRECT_PINS_RIGHT { { F1, F0, B0, C7 }, { F4, F5, F6, F7 } }`
   * If you want to specify a different direct pinout for the right half than the left half, you can define `DIRECT_PINS_RIGHT`. Currently, the size of `DIRECT_PINS` must be the same as `DIRECT_PINS_RIGHT`.

docs/custom_quantum_functions.md

@@ -144,6 +144,14 @@ This is useful for setting up stuff that you may need elsewhere, but isn't hardw
 * Keyboard/Revision: `void matrix_init_kb(void)`
 * Keymap: `void matrix_init_user(void)`
 
+### Low-level Matrix Overrides Function Documentation :id=low-level-matrix-overrides
+
+* GPIO pin initialisation: `void matrix_init_pins(void)`
+  * This needs to perform the low-level initialisation of all row and column pins. By default this will initialise the input/output state of each of the GPIO pins listed in `MATRIX_ROW_PINS` and `MATRIX_COL_PINS`, based on whether or not the keyboard is set up for `ROW2COL`, `COL2ROW`, or `DIRECT_PINS`. Should the keyboard designer override this function, no initialisation of pin state will occur within QMK itself, instead deferring to the keyboard's override.
+* `COL2ROW`-based row reads: `void matrix_read_rows_on_col(matrix_row_t current_matrix[], uint8_t current_col)`
+* `ROW2COL`-based column reads: `void matrix_read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row)`
+* `DIRECT_PINS`-based reads: `void matrix_read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row)`
+  * These three functions need to perform the low-level retrieval of matrix state of relevant input pins, based on the matrix type. Only one of the functions should be implemented, if needed. By default this will iterate through `MATRIX_ROW_PINS` and `MATRIX_COL_PINS`, configuring the inputs and outputs based on whether or not the keyboard is set up for `ROW2COL`, `COL2ROW`, or `DIRECT_PINS`. Should the keyboard designer override this function, no manipulation of matrix GPIO pin state will occur within QMK itself, instead deferring to the keyboard's override.
 
 ## Keyboard Post Initialization code
 

keyboards/anavi/macropad8/keymaps/default/config.h

@@ -0,0 +1,19 @@
+/* Copyright 2021 QMK
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#define LAYER_STATE_8BIT

keyboards/handwired/frankie_macropad/keymaps/default/config.h

@@ -0,0 +1,19 @@
+/* Copyright 2021 QMK
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#define LAYER_STATE_8BIT

keyboards/kbdfans/kbd67/rev1/keymaps/default/config.h

@@ -16,4 +16,4 @@
 
 #pragma once
 
-// place overrides here
+#define LAYER_STATE_8BIT

keyboards/mt84/keymaps/default/config.h

@@ -0,0 +1,19 @@
+/* Copyright 2021 QMK
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#define LAYER_STATE_8BIT

quantum/matrix.c

@@ -16,6 +16,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 #include <stdint.h>
 #include <stdbool.h>
+#include <string.h>
 #include "util.h"
 #include "matrix.h"
 #include "debounce.h"
@@ -24,14 +25,23 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #ifdef DIRECT_PINS
 static pin_t direct_pins[MATRIX_ROWS][MATRIX_COLS] = DIRECT_PINS;
 #elif (DIODE_DIRECTION == ROW2COL) || (DIODE_DIRECTION == COL2ROW)
+#    ifdef MATRIX_ROW_PINS
 static const pin_t row_pins[MATRIX_ROWS] = MATRIX_ROW_PINS;
+#    endif  // MATRIX_ROW_PINS
+#    ifdef MATRIX_COL_PINS
 static const pin_t col_pins[MATRIX_COLS] = MATRIX_COL_PINS;
+#    endif  // MATRIX_COL_PINS
 #endif
 
 /* matrix state(1:on, 0:off) */
 extern matrix_row_t raw_matrix[MATRIX_ROWS];  // raw values
 extern matrix_row_t matrix[MATRIX_ROWS];      // debounced values
 
+// user-defined overridable functions
+__attribute__((weak)) void matrix_init_pins(void);
+__attribute__((weak)) void matrix_read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row);
+__attribute__((weak)) void matrix_read_rows_on_col(matrix_row_t current_matrix[], uint8_t current_col);
+
 static inline void setPinOutput_writeLow(pin_t pin) {
     ATOMIC_BLOCK_FORCEON {
         setPinOutput(pin);
@@ -58,7 +68,7 @@ __attribute__((weak)) void matrix_init_pins(void) {
     }
 }
 
-__attribute__((weak)) bool matrix_read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row) {
+__attribute__((weak)) void matrix_read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row) {
     // Start with a clear matrix row
     matrix_row_t current_row_value = 0;
 
@@ -69,16 +79,13 @@ __attribute__((weak)) bool matrix_read_cols_on_row(matrix_row_t current_matrix[]
         }
     }
 
-    // If the row has changed, store the row and return the changed flag.
+    // Update the matrix
-    if (current_matrix[current_row] != current_row_value) {
+    current_matrix[current_row] = current_row_value;
-        current_matrix[current_row] = current_row_value;
-        return true;
-    }
-    return false;
 }
 
 #elif defined(DIODE_DIRECTION)
-#    if (DIODE_DIRECTION == COL2ROW)
+#    if defined(MATRIX_ROW_PINS) && defined(MATRIX_COL_PINS)
+#        if (DIODE_DIRECTION == COL2ROW)
 
 static void select_row(uint8_t row) { setPinOutput_writeLow(row_pins[row]); }
 
@@ -97,7 +104,7 @@ __attribute__((weak)) void matrix_init_pins(void) {
     }
 }
 
-__attribute__((weak)) bool matrix_read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row) {
+__attribute__((weak)) void matrix_read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row) {
     // Start with a clear matrix row
     matrix_row_t current_row_value = 0;
 
@@ -118,15 +125,11 @@ __attribute__((weak)) bool matrix_read_cols_on_row(matrix_row_t current_matrix[]
     unselect_row(current_row);
     matrix_output_unselect_delay();  // wait for all Col signals to go HIGH
 
-    // If the row has changed, store the row and return the changed flag.
+    // Update the matrix
-    if (current_matrix[current_row] != current_row_value) {
+    current_matrix[current_row] = current_row_value;
-        current_matrix[current_row] = current_row_value;
-        return true;
-    }
-    return false;
 }
 
-#    elif (DIODE_DIRECTION == ROW2COL)
+#        elif (DIODE_DIRECTION == ROW2COL)
 
 static void select_col(uint8_t col) { setPinOutput_writeLow(col_pins[col]); }
 
@@ -145,45 +148,32 @@ __attribute__((weak)) void matrix_init_pins(void) {
     }
 }
 
-__attribute__((weak)) bool matrix_read_rows_on_col(matrix_row_t current_matrix[], uint8_t current_col) {
+__attribute__((weak)) void matrix_read_rows_on_col(matrix_row_t current_matrix[], uint8_t current_col) {
-    bool matrix_changed = false;
-
     // Select col
     select_col(current_col);
     matrix_output_select_delay();
 
     // For each row...
     for (uint8_t row_index = 0; row_index < MATRIX_ROWS; row_index++) {
-        // Store last value of row prior to reading
-        matrix_row_t last_row_value    = current_matrix[row_index];
-        matrix_row_t current_row_value = last_row_value;
-
         // Check row pin state
         if (readPin(row_pins[row_index]) == 0) {
             // Pin LO, set col bit
-            current_row_value |= (MATRIX_ROW_SHIFTER << current_col);
+            current_matrix[row_index] |= (MATRIX_ROW_SHIFTER << current_col);
         } else {
             // Pin HI, clear col bit
-            current_row_value &= ~(MATRIX_ROW_SHIFTER << current_col);
+            current_matrix[row_index] &= ~(MATRIX_ROW_SHIFTER << current_col);
-        }
-
-        // Determine if the matrix changed state
-        if ((last_row_value != current_row_value)) {
-            matrix_changed |= true;
-            current_matrix[row_index] = current_row_value;
         }
     }
 
     // Unselect col
     unselect_col(current_col);
     matrix_output_unselect_delay();  // wait for all Row signals to go HIGH
-
-    return matrix_changed;
 }
 
-#    else
+#        else
-#        error DIODE_DIRECTION must be one of COL2ROW or ROW2COL!
+#            error DIODE_DIRECTION must be one of COL2ROW or ROW2COL!
-#    endif
+#        endif
+#    endif  // defined(MATRIX_ROW_PINS) && defined(MATRIX_COL_PINS)
 #else
 #    error DIODE_DIRECTION is not defined!
 #endif
@@ -204,20 +194,23 @@ void matrix_init(void) {
 }
 
 uint8_t matrix_scan(void) {
-    bool changed = false;
+    matrix_row_t curr_matrix[MATRIX_ROWS] = {0};
 
 #if defined(DIRECT_PINS) || (DIODE_DIRECTION == COL2ROW)
     // Set row, read cols
     for (uint8_t current_row = 0; current_row < MATRIX_ROWS; current_row++) {
-        changed |= matrix_read_cols_on_row(raw_matrix, current_row);
+        matrix_read_cols_on_row(curr_matrix, current_row);
     }
 #elif (DIODE_DIRECTION == ROW2COL)
     // Set col, read rows
     for (uint8_t current_col = 0; current_col < MATRIX_COLS; current_col++) {
-        changed |= matrix_read_rows_on_col(raw_matrix, current_col);
+        matrix_read_rows_on_col(curr_matrix, current_col);
     }
 #endif
 
+    bool changed = memcmp(raw_matrix, curr_matrix, sizeof(curr_matrix)) != 0;
+    if (changed) memcpy(raw_matrix, curr_matrix, sizeof(curr_matrix));
+
     debounce(raw_matrix, matrix, MATRIX_ROWS, changed);
 
     matrix_scan_quantum();

quantum/split_common/matrix.c

@@ -16,6 +16,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 #include <stdint.h>
 #include <stdbool.h>
+#include <string.h>
 #include "util.h"
 #include "matrix.h"
 #include "debounce.h"
@@ -31,8 +32,12 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #ifdef DIRECT_PINS
 static pin_t direct_pins[MATRIX_ROWS][MATRIX_COLS] = DIRECT_PINS;
 #elif (DIODE_DIRECTION == ROW2COL) || (DIODE_DIRECTION == COL2ROW)
+#    ifdef MATRIX_ROW_PINS
 static pin_t row_pins[MATRIX_ROWS] = MATRIX_ROW_PINS;
+#    endif  // MATRIX_ROW_PINS
+#    ifdef MATRIX_COL_PINS
 static pin_t col_pins[MATRIX_COLS] = MATRIX_COL_PINS;
+#    endif  // MATRIX_COL_PINS
 #endif
 
 /* matrix state(1:on, 0:off) */
@@ -45,6 +50,9 @@ uint8_t thisHand, thatHand;
 // user-defined overridable functions
 __attribute__((weak)) void matrix_slave_scan_kb(void) { matrix_slave_scan_user(); }
 __attribute__((weak)) void matrix_slave_scan_user(void) {}
+__attribute__((weak)) void matrix_init_pins(void);
+__attribute__((weak)) void matrix_read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row);
+__attribute__((weak)) void matrix_read_rows_on_col(matrix_row_t current_matrix[], uint8_t current_col);
 
 static inline void setPinOutput_writeLow(pin_t pin) {
     ATOMIC_BLOCK_FORCEON {
@@ -72,7 +80,7 @@ __attribute__((weak)) void matrix_init_pins(void) {
     }
 }
 
-__attribute__((weak)) bool matrix_read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row) {
+__attribute__((weak)) void matrix_read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row) {
     // Start with a clear matrix row
     matrix_row_t current_row_value = 0;
 
@@ -83,16 +91,13 @@ __attribute__((weak)) bool matrix_read_cols_on_row(matrix_row_t current_matrix[]
         }
     }
 
-    // If the row has changed, store the row and return the changed flag.
+    // Update the matrix
-    if (current_matrix[current_row] != current_row_value) {
+    current_matrix[current_row] = current_row_value;
-        current_matrix[current_row] = current_row_value;
-        return true;
-    }
-    return false;
 }
 
 #elif defined(DIODE_DIRECTION)
-#    if (DIODE_DIRECTION == COL2ROW)
+#    if defined(MATRIX_ROW_PINS) && defined(MATRIX_COL_PINS)
+#        if (DIODE_DIRECTION == COL2ROW)
 
 static void select_row(uint8_t row) { setPinOutput_writeLow(row_pins[row]); }
 
@@ -111,7 +116,7 @@ __attribute__((weak)) void matrix_init_pins(void) {
     }
 }
 
-__attribute__((weak)) bool matrix_read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row) {
+__attribute__((weak)) void matrix_read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row) {
     // Start with a clear matrix row
     matrix_row_t current_row_value = 0;
 
@@ -132,15 +137,11 @@ __attribute__((weak)) bool matrix_read_cols_on_row(matrix_row_t current_matrix[]
     unselect_row(current_row);
     matrix_output_unselect_delay();  // wait for all Col signals to go HIGH
 
-    // If the row has changed, store the row and return the changed flag.
+    // Update the matrix
-    if (current_matrix[current_row] != current_row_value) {
+    current_matrix[current_row] = current_row_value;
-        current_matrix[current_row] = current_row_value;
-        return true;
-    }
-    return false;
 }
 
-#    elif (DIODE_DIRECTION == ROW2COL)
+#        elif (DIODE_DIRECTION == ROW2COL)
 
 static void select_col(uint8_t col) { setPinOutput_writeLow(col_pins[col]); }
 
@@ -159,45 +160,32 @@ __attribute__((weak)) void matrix_init_pins(void) {
     }
 }
 
-__attribute__((weak)) bool matrix_read_rows_on_col(matrix_row_t current_matrix[], uint8_t current_col) {
+__attribute__((weak)) void matrix_read_rows_on_col(matrix_row_t current_matrix[], uint8_t current_col) {
-    bool matrix_changed = false;
-
     // Select col
     select_col(current_col);
     matrix_output_select_delay();
 
     // For each row...
     for (uint8_t row_index = 0; row_index < ROWS_PER_HAND; row_index++) {
-        // Store last value of row prior to reading
-        matrix_row_t last_row_value    = current_matrix[row_index];
-        matrix_row_t current_row_value = last_row_value;
-
         // Check row pin state
         if (readPin(row_pins[row_index]) == 0) {
             // Pin LO, set col bit
-            current_row_value |= (MATRIX_ROW_SHIFTER << current_col);
+            current_matrix[row_index] |= (MATRIX_ROW_SHIFTER << current_col);
         } else {
             // Pin HI, clear col bit
-            current_row_value &= ~(MATRIX_ROW_SHIFTER << current_col);
+            current_matrix[row_index] &= ~(MATRIX_ROW_SHIFTER << current_col);
-        }
-
-        // Determine if the matrix changed state
-        if ((last_row_value != current_row_value)) {
-            matrix_changed |= true;
-            current_matrix[row_index] = current_row_value;
         }
     }
 
     // Unselect col
     unselect_col(current_col);
     matrix_output_unselect_delay();  // wait for all Row signals to go HIGH
-
-    return matrix_changed;
 }
 
-#    else
+#        else
-#        error DIODE_DIRECTION must be one of COL2ROW or ROW2COL!
+#            error DIODE_DIRECTION must be one of COL2ROW or ROW2COL!
-#    endif
+#        endif
+#    endif  // defined(MATRIX_ROW_PINS) && defined(MATRIX_COL_PINS)
 #else
 #    error DIODE_DIRECTION is not defined!
 #endif
@@ -288,20 +276,23 @@ bool matrix_post_scan(void) {
 }
 
 uint8_t matrix_scan(void) {
-    bool local_changed = false;
+    matrix_row_t curr_matrix[MATRIX_ROWS] = {0};
 
 #if defined(DIRECT_PINS) || (DIODE_DIRECTION == COL2ROW)
     // Set row, read cols
     for (uint8_t current_row = 0; current_row < ROWS_PER_HAND; current_row++) {
-        local_changed |= matrix_read_cols_on_row(raw_matrix, current_row);
+        matrix_read_cols_on_row(curr_matrix, current_row);
     }
 #elif (DIODE_DIRECTION == ROW2COL)
     // Set col, read rows
     for (uint8_t current_col = 0; current_col < MATRIX_COLS; current_col++) {
-        local_changed |= matrix_read_rows_on_col(raw_matrix, current_col);
+        matrix_read_rows_on_col(curr_matrix, current_col);
     }
 #endif
 
+    bool local_changed = memcmp(raw_matrix, curr_matrix, sizeof(curr_matrix)) != 0;
+    if (local_changed) memcpy(raw_matrix, curr_matrix, sizeof(curr_matrix));
+
     debounce(raw_matrix, matrix + thisHand, ROWS_PER_HAND, local_changed);
 
     bool remote_changed = matrix_post_scan();