Keyboard: Helix serial improvements (#3608)

* add change_reciver2sender()/change_sender2reciver()

This is a change to improve readability.

* txled, rxled off in matrix_init()

* add serial_send_packet() / serial_recive_packet()

This is a change to reduce object size.

* add serial_low() at ISR() top

* add __attribute__((always_inline)) to some functions

* modify serial_send_packet()/serial_recive_packet()

A little, object size reduction.
A little, speedup.

* add debug code to helix/serial.c

* Adjust sampling timing of serial signal being received

* add split_scomm.c/split_scomm.h and change serial.c/serial.h

serial.c was divided into 2 layers, split_scom.c and serial.c.
The upper layer split_scomm.c is called from matrix.c.
The lower layer serial.c accesses the hardware.

* add split_scomm.c/split_scomm.h into helix/rev1

* reduce object size helix/rev2/matrix.c

* remove checksum check, add parity check

* force occur parity error for test

* parity test ok. remove test code

* change some comment & add skip code when buffer_size == 0

* serial.c: multiple types of transaction support

Add 4 bits transaction-type field at packet top.
Select Transaction Descriptor Table entry by transaction-type.

* helix serial master-slave transaction optimize

Using multi-type transaction feature of serial.c, communication contents between master slaves were optimized.

* add debug code for retry

* add comment into each config.h

* fix ISR status drop

* add a debug macro 'debug_retry_chg()'

* reduce led_test size

* remove debug code from helix/serial.c and etc.

* helix:five_rows change TAPPING_TERM value 140

* Improved compatibility with let's split of serial.c. Finish helix/serial.c improvement.

- The difference with the original let's split's serial.c
  - It's high-speed about 4 times.
  - Stable bi-directional data transfer. (Helix need master to slave transfer)
  - serial.h was divided 2 files, serial_config.h and sereial.h
  - With multiple types of transaction support, communication contents can be optimized. (NEW flexible API)

- USE OLD Simple APIs (compatible with let's split serial.c)
  - files :
    - serial_config.h -- hardware configuration (need include by config.h)
    - serial.c/serial.h -- serial communication

- USE NEW flexible APIs. (Support multi-type transaction function.)
  serial.c was divided into 2 layers, split_scom.c and serial.c.
  The upper layer split_scomm.c is called from matrix.c.
  The lower layer serial.c accesses the hardware.
  - files
    - split_scomm.c -- communication buffer is defined in here. call by matrix.c.
    - split_scomm.h -- buffer size is defined in here. include by matrix.c, split_util.c
    - serial_config.h -- hardware configuration (need include by config.h)
      To use the NEW API, specify #define SERIAL_USE_MULTI_TRANSACTION
    - serial.c/serial.h -- serial communication lower layer

- NEW APIs for serial.c / serial.h (The lower layer)
  // Soft Serial Transaction Descriptor
  typedef struct _SSTD_t  {
      uint8_t *status;
      uint8_t initiator2target_buffer_size;
      uint8_t *initiator2target_buffer;
      uint8_t target2initiator_buffer_size;
      uint8_t *target2initiator_buffer;
  } SSTD_t;

  // initiator is transaction start side
  void soft_serial_initiator_init(SSTD_t *sstd_table);
  // target is interrupt accept side
  void soft_serial_target_init(SSTD_t *sstd_table);
  int soft_serial_transaction(int sstd_index);
  int soft_serial_get_and_clean_target_status(int sstd_index);

- NEW APIs for split_scomm.c / split_scomm.h (The upper layer)
   move from old serial.c the following buffer and functions
     serial_slave_buffer[]
     serial_master_buffer[]
     void serial_master_init(void)
     void serial_slave_init(void)
     int serial_update_buffers(void)
   define SERIAL_xxxxx_BUFFER_LENGTH move from serial_config.h to split_scomm.h

keyboards/helix/rev1/keymaps/OLED_sample/serial_config.h

@@ -9,7 +9,7 @@
 #define SERIAL_PIN_INTERRUPT INT2_vect
 
 #define SERIAL_SLAVE_BUFFER_LENGTH MATRIX_ROWS/2
-#define SERIAL_MASTER_BUFFER_LENGTH 1
+#define SERIAL_MASTER_BUFFER_LENGTH 0
 
 //// #error rev1/keymaps/OLED_sample serial config
 

keyboards/helix/rev1/rules.mk

@@ -1,4 +1,4 @@
-SRC += rev1/matrix.c \
-	   rev1/split_util.c
+SRC += rev1/matrix.c
+SRC += rev1/split_util.c
 
 BACKLIGHT_ENABLE = no

keyboards/helix/rev1/serial_config.h

@@ -9,7 +9,7 @@
 #define SERIAL_PIN_INTERRUPT INT0_vect
 
 #define SERIAL_SLAVE_BUFFER_LENGTH MATRIX_ROWS/2
-#define SERIAL_MASTER_BUFFER_LENGTH 1
+#define SERIAL_MASTER_BUFFER_LENGTH 0
 
 /// #error rev1 serial config
 

keyboards/helix/rev2/keymaps/default/config.h

@@ -21,6 +21,9 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #ifndef CONFIG_USER_H
 #define CONFIG_USER_H
 
+// if you need more program area, try uncomment follow line
+//#include "serial_config_simpleapi.h"
+
 // place overrides here
 
 #endif /* CONFIG_USER_H */

keyboards/helix/rev2/keymaps/edvorakjp/config.h

@@ -1,6 +1,9 @@
 #ifndef CONFIG_USER_H
 #define CONFIG_USER_H
 
+// if you need more program area, try uncomment follow line
+//#include "serial_config_simpleapi.h"
+
 #undef TAPPING_FORCE_HOLD
 #undef TAPPING_TERM
 #define TAPPING_TERM 120

keyboards/helix/rev2/keymaps/five_rows/config.h

@@ -21,6 +21,12 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #ifndef CONFIG_USER_H
 #define CONFIG_USER_H
 
+// if you need more program area, try uncomment follow line
+//#include "serial_config_simpleapi.h"
+
+#undef TAPPING_TERM
+#define TAPPING_TERM 140
+
 // place overrides here
 
 #endif /* CONFIG_USER_H */

keyboards/helix/rev2/keymaps/five_rows_jis/config.h

@@ -23,6 +23,9 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 // place overrides here
 
+// if you need more program area, try uncomment follow line
+//#include "serial_config_simpleapi.h"
+
 #ifdef MOUSEKEY_ENABLE
   #undef MOUSEKEY_INTERVAL
   #define MOUSEKEY_INTERVAL 0

keyboards/helix/rev2/keymaps/froggy/config.h

@@ -21,6 +21,9 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #ifndef CONFIG_USER_H
 #define CONFIG_USER_H
 
+// if you need more program area, try uncomment follow line
+//#include "serial_config_simpleapi.h"
+
 #undef TAPPING_TERM
 #define TAPPING_TERM 200
 #define ONESHOT_TAP_TOGGLE 5 /* Tapping this number of times holds the key until tapped this number of times again. */

keyboards/helix/rev2/keymaps/led_test/config.h

@@ -21,6 +21,9 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #ifndef CONFIG_USER_H
 #define CONFIG_USER_H
 
+// if you need more program area, try uncomment follow line
+#include "serial_config_simpleapi.h"
+
 // place overrides here
 
 #endif /* CONFIG_USER_H */

keyboards/helix/rev2/matrix.c

@@ -20,6 +20,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 #include <stdint.h>
 #include <stdbool.h>
+#include <string.h>
 #include <avr/io.h>
 #include <avr/wdt.h>
 #include <avr/interrupt.h>
@@ -34,7 +35,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #ifdef USE_MATRIX_I2C
 #  include "i2c.h"
 #else // USE_SERIAL
-#  include "serial.h"
+#  include "split_scomm.h"
 #endif
 
 #ifndef DEBOUNCE
@@ -102,6 +103,8 @@ void matrix_init(void)
     init_cols();
 
     TX_RX_LED_INIT;
+    TXLED0;
+    RXLED0;
 
     // initialize matrix state: all keys off
     for (uint8_t i=0; i < MATRIX_ROWS; i++) {
@@ -178,17 +181,20 @@ int i2c_transaction(void) {
 
 #else // USE_SERIAL
 
-int serial_transaction(void) {
+int serial_transaction(int master_changed) {
     int slaveOffset = (isLeftHand) ? (ROWS_PER_HAND) : 0;
+#ifdef SERIAL_USE_MULTI_TRANSACTION
+    int ret=serial_update_buffers(master_changed);
+#else
     int ret=serial_update_buffers();
+#endif
     if (ret ) {
-        if(ret==2)RXLED1;
+        if(ret==2) RXLED1;
         return 1;
     }
-RXLED0;
-    for (int i = 0; i < ROWS_PER_HAND; ++i) {
-        matrix[slaveOffset+i] = serial_slave_buffer[i];
-    }
+    RXLED0;
+    memcpy(&matrix[slaveOffset],
+        (void *)serial_slave_buffer, sizeof(serial_slave_buffer));
     return 0;
 }
 #endif
@@ -199,19 +205,9 @@ uint8_t matrix_scan(void)
         matrix_master_scan();
     }else{
         matrix_slave_scan();
-
-//        if(serial_slave_DATA_CORRUPT()){
-//          TXLED0;
-          int offset = (isLeftHand) ? ROWS_PER_HAND : 0;
-
-          for (int i = 0; i < ROWS_PER_HAND; ++i) {
-              matrix[offset+i] = serial_master_buffer[i];
-          }
-
-//        }else{
-//          TXLED1;
-//        }
-
+        int offset = (isLeftHand) ? ROWS_PER_HAND : 0;
+        memcpy(&matrix[offset],
+               (void *)serial_master_buffer, sizeof(serial_master_buffer));
         matrix_scan_quantum();
     }
     return 1;
@@ -221,6 +217,7 @@ uint8_t matrix_scan(void)
 uint8_t matrix_master_scan(void) {
 
     int ret = _matrix_scan();
+    int mchanged = 1;
 
 #ifndef KEYBOARD_helix_rev1
     int offset = (isLeftHand) ? 0 : ROWS_PER_HAND;
@@ -231,16 +228,19 @@ uint8_t matrix_master_scan(void) {
 //        i2c_slave_buffer[i] = matrix[offset+i];
 //    }
 #else // USE_SERIAL
-    for (int i = 0; i < ROWS_PER_HAND; ++i) {
-        serial_master_buffer[i] = matrix[offset+i];
-    }
+  #ifdef SERIAL_USE_MULTI_TRANSACTION
+    mchanged = memcmp((void *)serial_master_buffer,
+		      &matrix[offset], sizeof(serial_master_buffer));
+  #endif
+    memcpy((void *)serial_master_buffer,
+	   &matrix[offset], sizeof(serial_master_buffer));
 #endif
 #endif
 
 #ifdef USE_MATRIX_I2C
     if( i2c_transaction() ) {
 #else // USE_SERIAL
-    if( serial_transaction() ) {
+    if( serial_transaction(mchanged) ) {
 #endif
         // turn on the indicator led when halves are disconnected
         TXLED1;
@@ -274,9 +274,19 @@ void matrix_slave_scan(void) {
         i2c_slave_buffer[i] = matrix[offset+i];
     }
 #else // USE_SERIAL
+  #ifdef SERIAL_USE_MULTI_TRANSACTION
+    int change = 0;
+  #endif
     for (int i = 0; i < ROWS_PER_HAND; ++i) {
+  #ifdef SERIAL_USE_MULTI_TRANSACTION
+        if( serial_slave_buffer[i] != matrix[offset+i] )
+	    change = 1;
+  #endif
         serial_slave_buffer[i] = matrix[offset+i];
     }
+  #ifdef SERIAL_USE_MULTI_TRANSACTION
+    slave_buffer_change_count += change;
+  #endif
 #endif
 }
 

keyboards/helix/rev2/rules.mk

@@ -1,3 +1,4 @@
-SRC += rev2/matrix.c \
-	   rev2/split_util.c \
-	   ws2812.c
+SRC += rev2/matrix.c
+SRC += rev2/split_util.c
+SRC += rev2/split_scomm.c
+SRC += ws2812.c

keyboards/helix/rev2/serial_config.h

@@ -8,8 +8,7 @@
 #define SERIAL_PIN_MASK _BV(PD2)
 #define SERIAL_PIN_INTERRUPT INT2_vect
 
-#define SERIAL_SLAVE_BUFFER_LENGTH MATRIX_ROWS/2
-#define SERIAL_MASTER_BUFFER_LENGTH MATRIX_ROWS/2
+#define SERIAL_USE_MULTI_TRANSACTION
 
 //// #error rev2 serial config
 

keyboards/helix/rev2/serial_config_simpleapi.h

@@ -0,0 +1,8 @@
+#ifndef SERIAL_CONFIG_SIMPLEAPI_H
+#define SERIAL_CONFIG_SIMPLEAPI_H
+
+#undef SERIAL_USE_MULTI_TRANSACTION
+#define SERIAL_SLAVE_BUFFER_LENGTH MATRIX_ROWS/2
+#define SERIAL_MASTER_BUFFER_LENGTH MATRIX_ROWS/2
+
+#endif // SERIAL_CONFIG_SIMPLEAPI_H

keyboards/helix/rev2/split_scomm.c

@@ -0,0 +1,73 @@
+#ifdef USE_SERIAL
+#ifdef SERIAL_USE_MULTI_TRANSACTION
+/* --- USE flexible API (using multi-type transaction function) --- */
+
+#include <stdbool.h>
+#include <stdint.h>
+#include <stddef.h>
+#include <split_scomm.h>
+#include "serial.h"
+#ifdef SERIAL_DEBUG_MODE
+#include <avr/io.h>
+#endif
+
+uint8_t volatile serial_slave_buffer[SERIAL_SLAVE_BUFFER_LENGTH] = {0};
+uint8_t volatile serial_master_buffer[SERIAL_MASTER_BUFFER_LENGTH] = {0};
+uint8_t volatile status_com = 0;
+uint8_t volatile status1 = 0;
+uint8_t slave_buffer_change_count = 0;
+uint8_t s_change_old = 0xff;
+
+SSTD_t transactions[] = {
+#define GET_SLAVE_STATUS 0
+    /* master buffer not changed, only recive slave_buffer_change_count */
+    { (uint8_t *)&status_com,
+      0, NULL,
+      sizeof(slave_buffer_change_count), &slave_buffer_change_count,
+    },
+#define PUT_MASTER_GET_SLAVE_STATUS 1
+    /* master buffer changed need send, and recive slave_buffer_change_count  */
+    { (uint8_t *)&status_com,
+      sizeof(serial_master_buffer), (uint8_t *)serial_master_buffer,
+      sizeof(slave_buffer_change_count), &slave_buffer_change_count,
+    },
+#define GET_SLAVE_BUFFER 2
+    /* recive serial_slave_buffer */
+    { (uint8_t *)&status1,
+      0, NULL,
+      sizeof(serial_slave_buffer), (uint8_t *)serial_slave_buffer
+    }
+};
+
+void serial_master_init(void)
+{
+    soft_serial_initiator_init(transactions);
+}
+
+void serial_slave_init(void)
+{
+    soft_serial_target_init(transactions);
+}
+
+// 0 => no error
+// 1 => slave did not respond
+// 2 => checksum error
+int serial_update_buffers(int master_update)
+{
+    int status;
+    static int need_retry = 0;
+    if( s_change_old != slave_buffer_change_count ) {
+	status = soft_serial_transaction(GET_SLAVE_BUFFER);
+	if( status == TRANSACTION_END )
+	    s_change_old = slave_buffer_change_count;
+    }
+    if( !master_update && !need_retry)
+	status = soft_serial_transaction(GET_SLAVE_STATUS);
+    else
+	status = soft_serial_transaction(PUT_MASTER_GET_SLAVE_STATUS);
+    need_retry = ( status == TRANSACTION_END ) ? 0 : 1;
+    return status;
+}
+
+#endif // SERIAL_USE_MULTI_TRANSACTION
+#endif /* USE_SERIAL */

keyboards/helix/rev2/split_scomm.h

@@ -0,0 +1,24 @@
+#ifndef SPLIT_COMM_H
+#define SPLIT_COMM_H
+
+#ifndef SERIAL_USE_MULTI_TRANSACTION
+/* --- USE Simple API (OLD API, compatible with let's split serial.c) --- */
+#include "serial.h"
+
+#else
+/* --- USE flexible API (using multi-type transaction function) --- */
+// Buffers for master - slave communication
+#define SERIAL_SLAVE_BUFFER_LENGTH MATRIX_ROWS/2
+#define SERIAL_MASTER_BUFFER_LENGTH MATRIX_ROWS/2
+
+extern volatile uint8_t serial_slave_buffer[SERIAL_SLAVE_BUFFER_LENGTH];
+extern volatile uint8_t serial_master_buffer[SERIAL_MASTER_BUFFER_LENGTH];
+extern uint8_t slave_buffer_change_count;
+
+void serial_master_init(void);
+void serial_slave_init(void);
+int serial_update_buffers(int master_changed);
+
+#endif
+
+#endif /* SPLIT_COMM_H */

keyboards/helix/rev2/split_util.c

@@ -11,7 +11,7 @@
 #ifdef USE_MATRIX_I2C
 #  include "i2c.h"
 #else
-#  include "serial.h"
+#  include "split_scomm.h"
 #endif
 
 volatile bool isLeftHand = true;

keyboards/helix/rules.mk

@@ -1,6 +1,9 @@
-SRC += i2c.c \
-	   serial.c \
-	   ssd1306.c
+SRC += i2c.c
+SRC += serial.c
+SRC += ssd1306.c
+
+# if firmware size over limit, try this option
+# CFLAGS += -flto
 
 # MCU name
 #MCU = at90usb1287