Split serial driver in generic protocol and hardware driver part

This allows other means of split communication to be implemented by
simply implementing the driver specific methods used in the serial protocol.

builddefs/common_features.mk

@@ -583,6 +583,14 @@ ifneq ($(strip $(DEBOUNCE_TYPE)), custom)
     QUANTUM_SRC += $(QUANTUM_DIR)/debounce/$(strip $(DEBOUNCE_TYPE)).c
 endif
 
+
+VALID_SERIAL_DRIVER_TYPES := bitbang usart
+
+SERIAL_DRIVER ?= bitbang
+ifeq ($(filter $(SERIAL_DRIVER),$(VALID_SERIAL_DRIVER_TYPES)),)
+    $(call CATASTROPHIC_ERROR,Invalid SERIAL_DRIVER,SERIAL_DRIVER="$(SERIAL_DRIVER)" is not a valid SERIAL driver)
+endif
+
 ifeq ($(strip $(SPLIT_KEYBOARD)), yes)
     POST_CONFIG_H += $(QUANTUM_DIR)/split_common/post_config.h
     OPT_DEFS += -DSPLIT_KEYBOARD
@@ -607,11 +615,11 @@ ifeq ($(strip $(SPLIT_KEYBOARD)), yes)
             endif
         endif
 
-        SERIAL_DRIVER ?= bitbang
         OPT_DEFS += -DSERIAL_DRIVER_$(strip $(shell echo $(SERIAL_DRIVER) | tr '[:lower:]' '[:upper:]'))
         ifeq ($(strip $(SERIAL_DRIVER)), bitbang)
             QUANTUM_LIB_SRC += serial.c
         else
+            QUANTUM_LIB_SRC += serial_protocol.c
             QUANTUM_LIB_SRC += serial_$(strip $(SERIAL_DRIVER)).c
         endif
     endif

docs/serial_driver.md

@@ -254,6 +254,16 @@ This is the default time window in milliseconds in which a successful communicat
 
 <hr>
 
+## Troubleshooting
+
+If you're having issues withe serial communication, you can enable debug messages that will give you insights which part of the communication failed. The enable these messages add to your keyboards `config.h` file:
+
+```c
+#define SERIAL_DEBUG
+```
+
+?> The messages will be printed out to the `CONSOLE` output. For additional information, refer to [Debugging/Troubleshooting QMK](faq_debug.md).
+
 ## Alternate Functions for selected STM32 MCUs
 
 Pins for USART Peripherals with 

drivers/serial.h

@@ -27,3 +27,13 @@ void soft_serial_initiator_init(void);
 void soft_serial_target_init(void);
 
 bool soft_serial_transaction(int sstd_index);
+
+#ifdef SERIAL_DEBUG
+#    include <debug.h>
+#    include <print.h>
+#    define serial_dprintf(...) dprintf(__VA_ARGS__)
+#else
+#    define serial_dprintf(...) \
+        do {                    \
+        } while (0)
+#endif

platforms/chibios/drivers/serial.c

@@ -233,7 +233,7 @@ static inline bool initiate_transaction(uint8_t sstd_index) {
     // check if the slave is present
     if (serial_read_pin()) {
         // slave failed to pull the line low, assume not present
-        dprintf("serial::NO_RESPONSE\n");
+        serial_dprintf("serial::NO_RESPONSE\n");
         chSysUnlock();
         return false;
     }
@@ -269,7 +269,7 @@ static inline bool initiate_transaction(uint8_t sstd_index) {
     serial_delay();
 
     if ((checksum_computed) != (checksum_received)) {
-        dprintf("serial::FAIL[%u,%u,%u]\n", checksum_computed, checksum_received, sstd_index);
+        serial_dprintf("serial::FAIL[%u,%u,%u]\n", checksum_computed, checksum_received, sstd_index);
         serial_output();
         serial_high();
 

platforms/chibios/drivers/serial_protocol.c

@@ -0,0 +1,164 @@
+// Copyright 2022 Stefan Kerkmann
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <ch.h>
+
+#include "quantum.h"
+#include "serial.h"
+#include "serial_protocol.h"
+#include "printf.h"
+#include "synchronization_util.h"
+
+static inline bool initiate_transaction(uint8_t transaction_id);
+static inline bool react_to_transaction(void);
+
+/**
+ * @brief This thread runs on the slave and responds to transactions initiated
+ * by the master.
+ */
+static THD_WORKING_AREA(waSlaveThread, 1024);
+static THD_FUNCTION(SlaveThread, arg) {
+    (void)arg;
+    chRegSetThreadName("split_protocol_tx_rx");
+
+    while (true) {
+        split_shared_memory_lock();
+        if (unlikely(!react_to_transaction())) {
+            /* Clear the receive queue, to start with a clean slate.
+             * Parts of failed transactions or spurious bytes could still be in it. */
+            serial_transport_driver_clear();
+        }
+        split_shared_memory_unlock();
+    }
+}
+
+/**
+ * @brief Slave specific initializations.
+ */
+void soft_serial_target_init(void) {
+    serial_transport_driver_slave_init();
+
+    /* Start transport thread. */
+    chThdCreateStatic(waSlaveThread, sizeof(waSlaveThread), HIGHPRIO, SlaveThread, NULL);
+}
+
+/**
+ * @brief Master specific initializations.
+ */
+void soft_serial_initiator_init(void) {
+    serial_transport_driver_master_init();
+}
+
+/**
+ * @brief React to transactions started by the master.
+ */
+static inline bool react_to_transaction(void) {
+    uint8_t transaction_id = 0;
+    /* Wait until there is a transaction for us. */
+    if (unlikely(!serial_transport_receive_blocking(&transaction_id, sizeof(transaction_id)))) {
+        return false;
+    }
+
+    /* Sanity check that we are actually responding to a valid transaction. */
+    if (unlikely(transaction_id >= NUM_TOTAL_TRANSACTIONS)) {
+        return false;
+    }
+
+    split_transaction_desc_t* transaction = &split_transaction_table[transaction_id];
+
+    /* Send back the handshake which is XORed as a simple checksum,
+     to signal that the slave is ready to receive possible transaction buffers  */
+    transaction_id ^= NUM_TOTAL_TRANSACTIONS;
+    if (unlikely(!serial_transport_send(&transaction_id, sizeof(transaction_id)))) {
+        return false;
+    }
+
+    /* Receive transaction buffer from the master. If this transaction requires it.*/
+    if (transaction->initiator2target_buffer_size) {
+        if (unlikely(!serial_transport_receive(split_trans_initiator2target_buffer(transaction), transaction->initiator2target_buffer_size))) {
+            return false;
+        }
+    }
+
+    /* Allow any slave processing to occur. */
+    if (transaction->slave_callback) {
+        transaction->slave_callback(transaction->initiator2target_buffer_size, split_trans_initiator2target_buffer(transaction), transaction->initiator2target_buffer_size, split_trans_target2initiator_buffer(transaction));
+    }
+
+    /* Send transaction buffer to the master. If this transaction requires it. */
+    if (transaction->target2initiator_buffer_size) {
+        if (unlikely(!serial_transport_send(split_trans_target2initiator_buffer(transaction), transaction->target2initiator_buffer_size))) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
+/**
+ * @brief Start transaction from the master half to the slave half.
+ *
+ * @param index Transaction Table index of the transaction to start.
+ * @return bool Indicates success of transaction.
+ */
+bool soft_serial_transaction(int index) {
+    split_shared_memory_lock();
+    bool result = initiate_transaction((uint8_t)index);
+    split_shared_memory_unlock();
+
+    if (unlikely(!result)) {
+        /* Clear the receive queue, to start with a clean slate.
+         * Parts of failed transactions or spurious bytes could still be in it. */
+        serial_transport_driver_clear();
+    }
+
+    return result;
+}
+
+/**
+ * @brief Initiate transaction to slave half.
+ */
+static inline bool initiate_transaction(uint8_t transaction_id) {
+    /* Sanity check that we are actually starting a valid transaction. */
+    if (unlikely(transaction_id >= NUM_TOTAL_TRANSACTIONS)) {
+        serial_dprintf("SPLIT: illegal transaction id\n");
+        return false;
+    }
+
+    split_transaction_desc_t* transaction = &split_transaction_table[transaction_id];
+
+    /* Send transaction table index to the slave, which doubles as basic handshake token. */
+    if (unlikely(!serial_transport_send(&transaction_id, sizeof(transaction_id)))) {
+        serial_dprintf("SPLIT: sending handshake failed\n");
+        return false;
+    }
+
+    uint8_t transaction_id_shake = 0xFF;
+
+    /* Which we always read back first so that we can error out correctly.
+     *   - due to the half duplex limitations on return codes, we always have to read *something*.
+     *   - without the read, write only transactions *always* succeed, even during the boot process where the slave is not ready.
+     */
+    if (unlikely(!serial_transport_receive(&transaction_id_shake, sizeof(transaction_id_shake)) || (transaction_id_shake != (transaction_id ^ NUM_TOTAL_TRANSACTIONS)))) {
+        serial_dprintf("SPLIT: receiving handshake failed\n");
+        return false;
+    }
+
+    /* Send transaction buffer to the slave. If this transaction requires it. */
+    if (transaction->initiator2target_buffer_size) {
+        if (unlikely(!serial_transport_send(split_trans_initiator2target_buffer(transaction), transaction->initiator2target_buffer_size))) {
+            serial_dprintf("SPLIT: sending buffer failed\n");
+            return false;
+        }
+    }
+
+    /* Receive transaction buffer from the slave. If this transaction requires it. */
+    if (transaction->target2initiator_buffer_size) {
+        if (unlikely(!serial_transport_receive(split_trans_target2initiator_buffer(transaction), transaction->target2initiator_buffer_size))) {
+            serial_dprintf("SPLIT: receiving buffer failed\n");
+            return false;
+        }
+    }
+
+    return true;
+}

platforms/chibios/drivers/serial_protocol.h

@@ -0,0 +1,49 @@
+// Copyright 2022 Stefan Kerkmann
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <stddef.h>
+#include <stdint.h>
+#include <stdbool.h>
+
+#pragma once
+
+/**
+ * @brief Clears any intermediate sending or receiving state of the driver to a known good
+ * state. This happens after errors in the middle of transactions, to start with
+ * a clean slate.
+ */
+void serial_transport_driver_clear(void);
+
+/**
+ * @brief Driver specific initialization on the slave half.
+ */
+void serial_transport_driver_slave_init(void);
+
+/**
+ * @brief Driver specific specific initialization on the master half.
+ */
+void serial_transport_driver_master_init(void);
+
+/**
+ * @brief  Blocking receive of size * bytes.
+ *
+ * @return true Receive success.
+ * @return false Receive failed, e.g. by bit errors.
+ */
+bool __attribute__((nonnull, hot)) serial_transport_receive(uint8_t* destination, const size_t size);
+
+/**
+ * @brief Blocking receive of size * bytes with an implicitly defined timeout.
+ *
+ * @return true Receive success.
+ * @return false Receive failed, e.g. by timeout or bit errors.
+ */
+bool __attribute__((nonnull, hot)) serial_transport_receive_blocking(uint8_t* destination, const size_t size);
+
+/**
+ * @brief Blocking send of buffer with timeout.
+ *
+ * @return true Send success.
+ * @return false Send failed, e.g. by timeout or bit errors.
+ */
+bool __attribute__((nonnull, hot)) serial_transport_send(const uint8_t* source, const size_t size);