[Keyboard] Helix add split common option (#7718)

* is_master, has_usb() move to rev2.[hc]

* Do recent helix/rev2 changes to helix/pico as well.

helix/pico/matrix.c: remove 'is_master'
helix/pico/pico.c: add 'is_master'
helix/pico/pico.h: add 'has_usb()' macro
helix/pico/split_util.c: remove 'setup_handedness()' 'has_usb()', add 'is_helix_master()' etc

* add HELIX=scan option into {rev2/pico}/local_features.mk

Made DEBUG_MATRIX_SCAN_RATE easy to use.

* Changed rules.mk to link "helix/local_drivers/ssd1306.c" only when OLED_ENABLE = yes.

* Added option to use split_common for helix/rev2, helix/pico keyboard.

how to build:
 ### build helix/pico (HelixPico) with helix current codes
  $ make helix/pico:KEY_MAP
  $ make helix/pico/back:KEY_MAP

 ### build helix/rev2 (Helix or Helix beta) with helix current codes
  $ make helix:KEY_MAP
  $ make helix/rev2/back:KEY_MAP
  $ make helix/rev2/under:KEY_MAP
  $ make helix/rev2/oled:KEY_MAP
  $ make helix/rev2/oled/back:KEY_MAP
  $ make helix/rev2/oled/under:KEY_MAP

 ### build helix/pico (HelixPico) with split_common codes
  $ make helix/pico/sc:KEY_MAP
  $ make helix/pico/sc/back:KEY_MAP
  $ make helix/pico/sc/under:KEY_MAP

 ### build helix/rev2 (Helix) with split_common codes
  $ make helix/rev2/sc:KEY_MAP
  $ make helix/rev2/sc/back:KEY_MAP
  $ make helix/rev2/sc/under:KEY_MAP
  $ make helix/rev2/sc/oled:KEY_MAP
  $ make helix/rev2/sc/oledback:KEY_MAP
  $ make helix/rev2/sc/oledunder:KEY_MAP

* add matrix_slave_scan_user() to helix/rev2/rev2.c, helix/pico/pico.h

* Changed 'helix:xulkal' to always use split_common and removed ad hoc code.

Added the following line to 'helix/rev2/keymaps/xulkal/rules.mk':

        SPLIT_KEYBOARD = yes

Removed the following ad hoc code from 'users/xulkal/custom_oled.c':

        #if KEYBOARD_helix_rev2
        extern uint8_t is_master;
        bool is_keyboard_master(void) { return is_master; }
        #endif

* add '#define DIODE_DIRECTION COL2ROW' into helix/{rev2|pico}/config.h

This commit does not change the build result.

* update helix readme

* keyboards/helix/readme.md
* keyboards/helix/pico/keymaps/default/readme.md
* keyboards/helix/rev2/keymaps/default/readme.md

Co-authored-by: mtei <2170248+mtei@users.noreply.github.com>

13 files changed, 145 insertions, 66 deletions

diff --git a/keyboards/helix/pico/config.h b/keyboards/helix/pico/config.h
index 260560f556..05713d7602 100644
--- a/keyboards/helix/pico/config.h
+++ b/keyboards/helix/pico/config.h
@@ -30,7 +30,6 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #define TAPPING_TERM 100
 
 /* Use I2C or Serial */
-#define USE_I2C
 #define USE_SERIAL
 //#define USE_MATRIX_I2C
 
@@ -60,6 +59,9 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #define MATRIX_COL_PINS { F4, F5, F6, F7, B1, B3, B2 }
 // #define MATRIX_COL_PINS { B2, B3, B1, F7, F6, F5, F4 } //uncomment this line and comment line above if you need to reverse left-to-right key order
 
+/* COL2ROW, ROW2COL*/
+#define DIODE_DIRECTION COL2ROW
+
 /* define if matrix has ghost */
 //#define MATRIX_HAS_GHOST
 
diff --git a/keyboards/helix/pico/keymaps/default/readme.md b/keyboards/helix/pico/keymaps/default/readme.md
index 9013e31287..3c097247e5 100644
--- a/keyboards/helix/pico/keymaps/default/readme.md
+++ b/keyboards/helix/pico/keymaps/default/readme.md
@@ -120,6 +120,13 @@ $ make HELIX=no_ani helix/pico/back:default  # with backlight without animation
 $ make helix/pico/under:default              # with underglow
 ```
 
+build (experimental use of split_common)
+```
+$ make helix/pico/sc:default
+$ make helix/pico/sc/back:default
+$ make helix/pico/sc/under:default
+```
+
 flash to keyboard
 ```
 $ make helix/pico:default:flash
diff --git a/keyboards/helix/pico/local_features.mk b/keyboards/helix/pico/local_features.mk
index 47b928647b..0277a3d227 100644
--- a/keyboards/helix/pico/local_features.mk
+++ b/keyboards/helix/pico/local_features.mk
@@ -10,7 +10,7 @@ define HELIX_CUSTOMISE_MSG
   $(info -  OLED_ENABLE          = $(OLED_ENABLE))
   $(info -  LED_BACK_ENABLE      = $(LED_BACK_ENABLE))
   $(info -  LED_UNDERGLOW_ENABLE = $(LED_UNDERGLOW_ENABLE))
-  $(info -  LED_ANIMATION        = $(LED_ANIMATIONS))
+  $(info -  LED_ANIMATIONS       = $(LED_ANIMATIONS))
   $(info -  IOS_DEVICE_ENABLE    = $(IOS_DEVICE_ENABLE))
   $(info )
 endef
@@ -43,12 +43,34 @@ endef
     ifeq ($(findstring ios,$(HELIX)), ios)
       IOS_DEVICE_ENABLE = yes
     endif
+    ifeq ($(findstring scan,$(HELIX)), scan)
+      # use DEBUG_MATRIX_SCAN_RATE
+      # see docs/newbs_testing_debugging.md
+      OPT_DEFS +=  -DDEBUG_MATRIX_SCAN_RATE
+      CONSOLE_ENABLE = yes
+      SHOW_VERBOSE_INFO = yes
+    endif
     ifeq ($(findstring verbose,$(HELIX)), verbose)
-       SHOW_VERBOSE_INFO = yes
+      SHOW_VERBOSE_INFO = yes
     endif
     SHOW_HELIX_OPTIONS = yes
   endif
 
+ifneq ($(strip $(SPLIT_KEYBOARD)), yes)
+  SRC += local_drivers/serial.c
+  KEYBOARD_PATHS += $(HELIX_TOP_DIR)/local_drivers
+
+  # A workaround until #7089 is merged.
+  #   serial.c must not be compiled with the -lto option.
+  #   The current LIB_SRC has a side effect with the -fno-lto option, so use it.
+  LIB_SRC += local_drivers/serial.c
+
+  CUSTOM_MATRIX = yes
+
+  SRC += pico/matrix.c
+  SRC += pico/split_util.c
+endif
+
 ########
 # convert Helix-specific options (that represent combinations of standard options)
 #   into QMK standard options.
@@ -73,11 +95,13 @@ ifeq ($(strip $(LED_ANIMATIONS)), yes)
 endif
 
 ifeq ($(strip $(OLED_ENABLE)), yes)
+    SRC += local_drivers/i2c.c
+    SRC += local_drivers/ssd1306.c
+    KEYBOARD_PATHS += $(HELIX_TOP_DIR)/local_drivers
     OPT_DEFS += -DOLED_ENABLE
-endif
-
-ifeq ($(strip $(LOCAL_GLCDFONT)), yes)
-    OPT_DEFS += -DLOCAL_GLCDFONT
+    ifeq ($(strip $(LOCAL_GLCDFONT)), yes)
+        OPT_DEFS += -DLOCAL_GLCDFONT
+    endif
 endif
 
 ifeq ($(strip $(AUDIO_ENABLE)),yes)
@@ -92,8 +116,10 @@ endif
 ifneq ($(strip $(SHOW_HELIX_OPTIONS)),)
   $(eval $(call HELIX_CUSTOMISE_MSG))
   ifneq ($(strip $(SHOW_VERBOSE_INFO)),)
-     $(info -- RGBLIGHT_ENABLE = $(RGBLIGHT_ENABLE))
-     $(info -- OPT_DEFS        = $(OPT_DEFS))
+     $(info -- RGBLIGHT_ENABLE    = $(RGBLIGHT_ENABLE))
+     $(info -- OLED_DRIVER_ENABLE = $(OLED_DRIVER_ENABLE))
+     $(info -- CONSOLE_ENABLE     = $(CONSOLE_ENABLE))
+     $(info -- OPT_DEFS           = $(OPT_DEFS))
      $(info -- LINK_TIME_OPTIMIZATION_ENABLE = $(LINK_TIME_OPTIMIZATION_ENABLE))
      $(info )
   endif
diff --git a/keyboards/helix/pico/matrix.c b/keyboards/helix/pico/matrix.c
index c2940e3b3e..a537ef03c4 100644
--- a/keyboards/helix/pico/matrix.c
+++ b/keyboards/helix/pico/matrix.c
@@ -46,7 +46,6 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 static uint8_t debouncing = DEBOUNCE;
 static const int ROWS_PER_HAND = MATRIX_ROWS/2;
 static uint8_t error_count = 0;
-uint8_t is_master = 0 ;
 
 static const uint8_t row_pins[MATRIX_ROWS] = MATRIX_ROW_PINS;
 static const uint8_t col_pins[MATRIX_COLS] = MATRIX_COL_PINS;
@@ -94,9 +93,8 @@ uint8_t matrix_cols(void)
 
 void matrix_init(void)
 {
-    debug_enable = true;
-    debug_matrix = true;
-    debug_mouse = true;
+    split_keyboard_setup();
+
     // initialize row and col
     unselect_rows();
     init_cols();
@@ -111,8 +109,6 @@ void matrix_init(void)
         matrix_debouncing[i] = 0;
     }
 
-    is_master = has_usb();
-
     matrix_init_quantum();
 }
 
@@ -197,7 +193,7 @@ int serial_transaction(void) {
 
 uint8_t matrix_scan(void)
 {
-    if (is_master) {
+    if (is_helix_master()) {
         matrix_master_scan();
     }else{
         matrix_slave_scan();
diff --git a/keyboards/helix/pico/pico.c b/keyboards/helix/pico/pico.c
index bb8ba9ca2b..12b8ae9efa 100644
--- a/keyboards/helix/pico/pico.c
+++ b/keyboards/helix/pico/pico.c
@@ -1,5 +1,9 @@
 #include "helix.h"
 
+// Each keymap.c should use is_keyboard_master() instead of 'is_master'.
+// But keep 'is_master' for a while for backwards compatibility
+//  for the old keymap.c.
+uint8_t is_master = false;
 
 #ifdef SSD1306OLED
 #include "ssd1306.h"
@@ -15,6 +19,23 @@ void led_set_kb(uint8_t usb_led) {
 #endif
 
 void matrix_init_kb(void) {
+    // Each keymap.c should use is_keyboard_master() instead of is_master.
+    // But keep is_master for a while for backwards compatibility
+    //  for the old keymap.c.
+    is_master = is_keyboard_master();
 
 	matrix_init_user();
 };
+
+void keyboard_post_init_kb(void) {
+#if defined(DEBUG_MATRIX_SCAN_RATE)
+    debug_enable = true;
+#endif
+    keyboard_post_init_user();
+}
+
+#if defined(SPLIT_KEYBOARD) && defined(SSD1306OLED)
+void matrix_slave_scan_user(void) {
+    matrix_scan_user();
+}
+#endif
diff --git a/keyboards/helix/pico/pico.h b/keyboards/helix/pico/pico.h
index 4360be2a9c..303fe315ea 100644
--- a/keyboards/helix/pico/pico.h
+++ b/keyboards/helix/pico/pico.h
@@ -4,18 +4,16 @@
 
 #include "quantum.h"
 
-#ifdef RGBLIGHT_ENABLE
-//rgb led driver
-#include "ws2812.h"
+#ifndef SPLIT_KEYBOARD
+  extern bool is_helix_master(void);
+  #define is_keyboard_master() is_helix_master()
 #endif
 
-#ifdef USE_I2C
-#include <stddef.h>
-#ifdef __AVR__
-	#include <avr/io.h>
-	#include <avr/interrupt.h>
-#endif
-#endif
+// Each keymap.c should use is_keyboard_master() instead of 'is_master', 'has_usb()'.
+// But keep 'is_master' for a while for backwards compatibility
+//  for the old keymap.c.
+extern uint8_t is_master; // 'is_master' will be obsolete, it is recommended to use 'is_keyboard_master ()' instead.
+#define has_usb() is_keyboard_master()
 
 #ifndef FLIP_HALF
 // Standard Keymap
diff --git a/keyboards/helix/pico/post_config.h b/keyboards/helix/pico/post_config.h
new file mode 100644
index 0000000000..dda73d5d22
--- /dev/null
+++ b/keyboards/helix/pico/post_config.h
@@ -0,0 +1,7 @@
+#pragma once
+
+#if defined(SPLIT_KEYBOARD) /* if use split_common */
+#  if defined(RGBLIGHT_ENABLE) && !defined(RGBLIGHT_SPLIT)
+#    define RGBLIGHT_SPLIT /* helix hardware need this */
+#  endif
+#endif
diff --git a/keyboards/helix/pico/rules.mk b/keyboards/helix/pico/rules.mk
index d3ad20ccad..cb9a70e00e 100644
--- a/keyboards/helix/pico/rules.mk
+++ b/keyboards/helix/pico/rules.mk
@@ -1,20 +1,5 @@
 KEYBOARD_LOCAL_FEATURES_MK := $(dir $(lastword $(MAKEFILE_LIST)))local_features.mk
 
-SRC += local_drivers/i2c.c
-SRC += local_drivers/serial.c
-SRC += local_drivers/ssd1306.c
-KEYBOARD_PATHS += $(HELIX_TOP_DIR)/local_drivers
-
-# A workaround until #7089 is merged.
-#   serial.c must not be compiled with the -lto option.
-#   The current LIB_SRC has a side effect with the -fno-lto option, so use it.
-LIB_SRC += local_drivers/serial.c
-
-CUSTOM_MATRIX = yes
-
-SRC += pico/matrix.c
-SRC += pico/split_util.c
-
 # Helix Spacific Build Options default values
 OLED_ENABLE = no            # OLED_ENABLE
 LOCAL_GLCDFONT = no         # use each keymaps "helixfont.h" insted of "common/glcdfont.c"
diff --git a/keyboards/helix/pico/sc/back/rules.mk b/keyboards/helix/pico/sc/back/rules.mk
new file mode 100644
index 0000000000..066fffb74a
--- /dev/null
+++ b/keyboards/helix/pico/sc/back/rules.mk
@@ -0,0 +1 @@
+LED_BACK_ENABLE = yes
diff --git a/keyboards/helix/pico/sc/rules.mk b/keyboards/helix/pico/sc/rules.mk
new file mode 100644
index 0000000000..d38a618090
--- /dev/null
+++ b/keyboards/helix/pico/sc/rules.mk
@@ -0,0 +1 @@
+SPLIT_KEYBOARD = yes
diff --git a/keyboards/helix/pico/sc/under/rules.mk b/keyboards/helix/pico/sc/under/rules.mk
new file mode 100644
index 0000000000..a37aa6fab3
--- /dev/null
+++ b/keyboards/helix/pico/sc/under/rules.mk
@@ -0,0 +1 @@
+LED_UNDERGLOW_ENABLE = yes
diff --git a/keyboards/helix/pico/split_util.c b/keyboards/helix/pico/split_util.c
index beb39fa005..c77e63f33c 100644
--- a/keyboards/helix/pico/split_util.c
+++ b/keyboards/helix/pico/split_util.c
@@ -7,6 +7,7 @@
 #include "split_util.h"
 #include "matrix.h"
 #include "keyboard.h"
+#include "wait.h"
 
 #ifdef USE_MATRIX_I2C
 #  include "i2c.h"
@@ -14,21 +15,65 @@
 #  include "serial.h"
 #endif
 
+#ifdef EE_HANDS
+#    include "eeconfig.h"
+#endif
+
+#ifndef SPLIT_USB_TIMEOUT
+  #define SPLIT_USB_TIMEOUT 2500
+#endif
+
 volatile bool isLeftHand = true;
 
-static void setup_handedness(void) {
-  #ifdef EE_HANDS
-    isLeftHand = eeprom_read_byte(EECONFIG_HANDEDNESS);
-  #else
-    // I2C_MASTER_RIGHT is deprecated, use MASTER_RIGHT instead, since this works for both serial and i2c
-    #if defined(I2C_MASTER_RIGHT) || defined(MASTER_RIGHT)
-      isLeftHand = !has_usb();
-    #else
-      isLeftHand = has_usb();
-    #endif
-  #endif
+bool waitForUsb(void) {
+    for (uint8_t i = 0; i < (SPLIT_USB_TIMEOUT / 100); i++) {
+        // This will return true of a USB connection has been established
+        if (UDADDR & _BV(ADDEN)) {
+            return true;
+        }
+        wait_ms(100);
+    }
+
+    // Avoid NO_USB_STARTUP_CHECK - Disable USB as the previous checks seem to enable it somehow
+    (USBCON &= ~(_BV(USBE) | _BV(OTGPADE)));
+
+    return false;
+}
+
+bool is_keyboard_left(void) {
+#if defined(SPLIT_HAND_PIN)
+    // Test pin SPLIT_HAND_PIN for High/Low, if low it's right hand
+    setPinInput(SPLIT_HAND_PIN);
+    return readPin(SPLIT_HAND_PIN);
+#elif defined(EE_HANDS)
+    return eeconfig_read_handedness();
+#elif defined(MASTER_RIGHT)
+    return !is_helix_master();
+#endif
+
+    return is_helix_master();
 }
 
+bool is_helix_master(void) {
+    static enum { UNKNOWN, MASTER, SLAVE } usbstate = UNKNOWN;
+
+    // only check once, as this is called often
+    if (usbstate == UNKNOWN) {
+#if defined(SPLIT_USB_DETECT)
+        usbstate = waitForUsb() ? MASTER : SLAVE;
+#elif defined(__AVR__)
+        USBCON |= (1 << OTGPADE);  // enables VBUS pad
+        wait_us(5);
+
+        usbstate = (USBSTA & (1 << VBUS)) ? MASTER : SLAVE;  // checks state of VBUS
+#else
+        usbstate = MASTER;
+#endif
+    }
+
+    return (usbstate == MASTER);
+ }
+
 static void keyboard_master_setup(void) {
 
 #ifdef USE_MATRIX_I2C
@@ -47,24 +92,13 @@ static void keyboard_slave_setup(void) {
 #endif
 }
 
-bool has_usb(void) {
-   USBCON |= (1 << OTGPADE); //enables VBUS pad
-   _delay_us(5);
-   return (USBSTA & (1<<VBUS));  //checks state of VBUS
-}
-
 void split_keyboard_setup(void) {
-   setup_handedness();
+   isLeftHand = is_keyboard_left();
 
-   if (has_usb()) {
+   if (is_helix_master()) {
       keyboard_master_setup();
    } else {
       keyboard_slave_setup();
    }
    sei();
 }
-
-// this code runs before the usb and keyboard is initialized
-void matrix_setup(void) {
-    split_keyboard_setup();
-}
diff --git a/keyboards/helix/pico/split_util.h b/keyboards/helix/pico/split_util.h
index 687ca19bd3..c0ecdb0974 100644
--- a/keyboards/helix/pico/split_util.h
+++ b/keyboards/helix/pico/split_util.h
@@ -12,7 +12,7 @@ extern volatile bool isLeftHand;
 void matrix_slave_scan(void);
 
 void split_keyboard_setup(void);
-bool has_usb(void);
+bool is_helix_master(void);
 
 void matrix_master_OLED_init (void);