diff --git a/builddefs/common_features.mk b/builddefs/common_features.mk index ff0c6b0cc6e..83505596d2d 100644 --- a/builddefs/common_features.mk +++ b/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 diff --git a/docs/serial_driver.md b/docs/serial_driver.md index 3338b55fec6..7c0daec9b17 100644 --- a/docs/serial_driver.md +++ b/docs/serial_driver.md @@ -254,6 +254,16 @@ This is the default time window in milliseconds in which a successful communicat
+## 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 diff --git a/drivers/serial.h b/drivers/serial.h index 0cfdbd99594..fb91b136e7c 100644 --- a/drivers/serial.h +++ b/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 +# include +# define serial_dprintf(...) dprintf(__VA_ARGS__) +#else +# define serial_dprintf(...) \ + do { \ + } while (0) +#endif diff --git a/platforms/chibios/drivers/serial.c b/platforms/chibios/drivers/serial.c index 0cff057d1db..e5f346ba337 100644 --- a/platforms/chibios/drivers/serial.c +++ b/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(); diff --git a/platforms/chibios/drivers/serial_protocol.c b/platforms/chibios/drivers/serial_protocol.c new file mode 100644 index 00000000000..3e160e0c5c2 --- /dev/null +++ b/platforms/chibios/drivers/serial_protocol.c @@ -0,0 +1,164 @@ +// Copyright 2022 Stefan Kerkmann +// SPDX-License-Identifier: GPL-2.0-or-later + +#include + +#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; +} diff --git a/platforms/chibios/drivers/serial_protocol.h b/platforms/chibios/drivers/serial_protocol.h new file mode 100644 index 00000000000..4275a7f8d8d --- /dev/null +++ b/platforms/chibios/drivers/serial_protocol.h @@ -0,0 +1,49 @@ +// Copyright 2022 Stefan Kerkmann +// SPDX-License-Identifier: GPL-2.0-or-later + +#include +#include +#include + +#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); diff --git a/platforms/chibios/drivers/serial_usart.c b/platforms/chibios/drivers/serial_usart.c index 7ea25d005bc..f76afb5db40 100644 --- a/platforms/chibios/drivers/serial_usart.c +++ b/platforms/chibios/drivers/serial_usart.c @@ -3,6 +3,7 @@ // SPDX-License-Identifier: GPL-2.0-or-later #include "serial_usart.h" +#include "serial_protocol.h" #include "synchronization_util.h" #if defined(SERIAL_USART_CONFIG) @@ -26,14 +27,6 @@ static QMKSerialConfig serial_config = { static QMKSerialDriver* serial_driver = (QMKSerialDriver*)&SERIAL_USART_DRIVER; -static inline bool react_to_transactions(void); -static inline bool __attribute__((nonnull)) receive(uint8_t* destination, const size_t size); -static inline bool __attribute__((nonnull)) receive_blocking(uint8_t* destination, const size_t size); -static inline bool __attribute__((nonnull)) send(const uint8_t* source, const size_t size); -static inline bool initiate_transaction(uint8_t sstd_index); -static inline void usart_clear(void); -static inline void usart_driver_start(void); - #if HAL_USE_SERIAL /** @@ -43,10 +36,7 @@ static inline void usart_driver_start(void) { sdStart(serial_driver, &serial_config); } -/** - * @brief Clear the receive input queue. - */ -static inline void usart_clear(void) { +inline void serial_transport_driver_clear(void) { osalSysLock(); bool volatile queue_not_empty = !iqIsEmptyI(&serial_driver->iqueue); osalSysUnlock(); @@ -89,10 +79,7 @@ static inline void usart_driver_start(void) { sioStartOperation(serial_driver, &serial_usart_operation); } -/** - * @brief Clear the receive input queue, as some MCUs have built-in hardware FIFOs. - */ -static inline void usart_clear(void) { +inline void serial_transport_driver_clear(void) { osalSysLock(); while (!sioIsRXEmptyX(serial_driver)) { (void)sioGetX(serial_driver); @@ -106,13 +93,7 @@ static inline void usart_clear(void) { #endif -/** - * @brief Blocking send of buffer with timeout. - * - * @return true Send success. - * @return false Send failed. - */ -static inline bool send(const uint8_t* source, const size_t size) { +inline bool serial_transport_send(const uint8_t* source, const size_t size) { bool success = (size_t)chnWriteTimeout(serial_driver, source, size, TIME_MS2I(SERIAL_USART_TIMEOUT)) == size; #if !defined(SERIAL_USART_FULL_DUPLEX) @@ -129,13 +110,13 @@ static inline bool send(const uint8_t* source, const size_t size) { uint8_t dump[64]; while (unlikely(bytes_left >= 64)) { - if (unlikely(!receive(dump, 64))) { + if (unlikely(!serial_transport_receive(dump, 64))) { return false; } bytes_left -= 64; } - return receive(dump, bytes_left); + return serial_transport_receive(dump, bytes_left); # else /* The SIO driver directly accesses the hardware FIFOs of the USART * peripheral. As these are limited in depth, the RX FIFO might have been @@ -159,24 +140,12 @@ static inline bool send(const uint8_t* source, const size_t size) { return success; } -/** - * @brief Blocking receive of size * bytes with timeout. - * - * @return true Receive success. - * @return false Receive failed, e.g. by timeout. - */ -static inline bool receive(uint8_t* destination, const size_t size) { +inline bool serial_transport_receive(uint8_t* destination, const size_t size) { bool success = (size_t)chnReadTimeout(serial_driver, destination, size, TIME_MS2I(SERIAL_USART_TIMEOUT)) == size; return success; } -/** - * @brief Blocking receive of size * bytes. - * - * @return true Receive success. - * @return false Receive failed. - */ -static inline bool receive_blocking(uint8_t* destination, const size_t size) { +inline bool serial_transport_receive_blocking(uint8_t* destination, const size_t size) { bool success = (size_t)chnRead(serial_driver, destination, size) == size; return success; } @@ -243,86 +212,12 @@ __attribute__((weak, nonnull)) void usart_slave_init(QMKSerialDriver** driver) { usart_init(); } -/** - * @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("usart_tx_rx"); - - while (true) { - if (unlikely(!react_to_transactions())) { - /* Clear the receive queue, to start with a clean slate. - * Parts of failed transactions or spurious bytes could still be in it. */ - usart_clear(); - } - split_shared_memory_unlock(); - } -} - -/** - * @brief Slave specific initializations. - */ -void soft_serial_target_init(void) { +void serial_transport_driver_slave_init(void) { usart_slave_init(&serial_driver); - usart_driver_start(); - - /* Start transport thread. */ - chThdCreateStatic(waSlaveThread, sizeof(waSlaveThread), HIGHPRIO, SlaveThread, NULL); } -/** - * @brief React to transactions started by the master. - */ -static inline bool react_to_transactions(void) { - /* Wait until there is a transaction for us. */ - uint8_t sstd_index = 0; - receive_blocking(&sstd_index, sizeof(sstd_index)); - - /* Sanity check that we are actually responding to a valid transaction. */ - if (unlikely(sstd_index >= NUM_TOTAL_TRANSACTIONS)) { - return false; - } - - split_shared_memory_lock(); - split_transaction_desc_t* trans = &split_transaction_table[sstd_index]; - - /* Send back the handshake which is XORed as a simple checksum, - to signal that the slave is ready to receive possible transaction buffers */ - sstd_index ^= HANDSHAKE_MAGIC; - if (unlikely(!send(&sstd_index, sizeof(sstd_index)))) { - return false; - } - - /* Receive transaction buffer from the master. If this transaction requires it.*/ - if (trans->initiator2target_buffer_size) { - if (unlikely(!receive(split_trans_initiator2target_buffer(trans), trans->initiator2target_buffer_size))) { - return false; - } - } - - /* Allow any slave processing to occur. */ - if (trans->slave_callback) { - trans->slave_callback(trans->initiator2target_buffer_size, split_trans_initiator2target_buffer(trans), trans->initiator2target_buffer_size, split_trans_target2initiator_buffer(trans)); - } - - /* Send transaction buffer to the master. If this transaction requires it. */ - if (trans->target2initiator_buffer_size) { - if (unlikely(!send(split_trans_target2initiator_buffer(trans), trans->target2initiator_buffer_size))) { - return false; - } - } - - return true; -} - -/** - * @brief Master specific initializations. - */ -void soft_serial_initiator_init(void) { +void serial_transport_driver_master_init(void) { usart_master_init(&serial_driver); #if defined(MCU_STM32) && defined(SERIAL_USART_PIN_SWAP) @@ -331,69 +226,3 @@ void soft_serial_initiator_init(void) { usart_driver_start(); } - -/** - * @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) { - /* Clear the receive queue, to start with a clean slate. - * Parts of failed transactions or spurious bytes could still be in it. */ - usart_clear(); - - split_shared_memory_lock(); - bool result = initiate_transaction((uint8_t)index); - split_shared_memory_unlock(); - - return result; -} - -/** - * @brief Initiate transaction to slave half. - */ -static inline bool initiate_transaction(uint8_t sstd_index) { - /* Sanity check that we are actually starting a valid transaction. */ - if (unlikely(sstd_index >= NUM_TOTAL_TRANSACTIONS)) { - dprintln("USART: Illegal transaction Id."); - return false; - } - - split_transaction_desc_t* trans = &split_transaction_table[sstd_index]; - - /* Send transaction table index to the slave, which doubles as basic handshake token. */ - if (unlikely(!send(&sstd_index, sizeof(sstd_index)))) { - dprintln("USART: Send Handshake failed."); - return false; - } - - uint8_t sstd_index_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(!receive(&sstd_index_shake, sizeof(sstd_index_shake)) || (sstd_index_shake != (sstd_index ^ HANDSHAKE_MAGIC)))) { - dprintln("USART: Handshake failed."); - return false; - } - - /* Send transaction buffer to the slave. If this transaction requires it. */ - if (trans->initiator2target_buffer_size) { - if (unlikely(!send(split_trans_initiator2target_buffer(trans), trans->initiator2target_buffer_size))) { - dprintln("USART: Send failed."); - return false; - } - } - - /* Receive transaction buffer from the slave. If this transaction requires it. */ - if (trans->target2initiator_buffer_size) { - if (unlikely(!receive(split_trans_target2initiator_buffer(trans), trans->target2initiator_buffer_size))) { - dprintln("USART: Receive failed."); - return false; - } - } - - return true; -} diff --git a/platforms/chibios/drivers/serial_usart.h b/platforms/chibios/drivers/serial_usart.h index 98c634dac07..fa062cd736b 100644 --- a/platforms/chibios/drivers/serial_usart.h +++ b/platforms/chibios/drivers/serial_usart.h @@ -5,11 +5,46 @@ #include "quantum.h" #include "serial.h" -#include "printf.h" - -#include #include +#if defined(SOFT_SERIAL_PIN) +# define SERIAL_USART_TX_PIN SOFT_SERIAL_PIN +#endif + +#if !defined(SERIAL_USART_TX_PIN) +# define SERIAL_USART_TX_PIN A9 +#endif + +#if !defined(SERIAL_USART_RX_PIN) +# define SERIAL_USART_RX_PIN A10 +#endif + +#if !defined(SELECT_SOFT_SERIAL_SPEED) +# define SELECT_SOFT_SERIAL_SPEED 1 +#endif + +#if defined(SERIAL_USART_SPEED) +// Allow advanced users to directly set SERIAL_USART_SPEED +#elif SELECT_SOFT_SERIAL_SPEED == 0 +# define SERIAL_USART_SPEED 460800 +#elif SELECT_SOFT_SERIAL_SPEED == 1 +# define SERIAL_USART_SPEED 230400 +#elif SELECT_SOFT_SERIAL_SPEED == 2 +# define SERIAL_USART_SPEED 115200 +#elif SELECT_SOFT_SERIAL_SPEED == 3 +# define SERIAL_USART_SPEED 57600 +#elif SELECT_SOFT_SERIAL_SPEED == 4 +# define SERIAL_USART_SPEED 38400 +#elif SELECT_SOFT_SERIAL_SPEED == 5 +# define SERIAL_USART_SPEED 19200 +#else +# error invalid SELECT_SOFT_SERIAL_SPEED value +#endif + +#if !defined(SERIAL_USART_TIMEOUT) +# define SERIAL_USART_TIMEOUT 20 +#endif + #if HAL_USE_SERIAL typedef SerialDriver QMKSerialDriver; @@ -40,18 +75,6 @@ typedef SIOConfig QMKSerialConfig; # endif #endif -#if defined(SOFT_SERIAL_PIN) -# define SERIAL_USART_TX_PIN SOFT_SERIAL_PIN -#endif - -#if !defined(SERIAL_USART_TX_PIN) -# define SERIAL_USART_TX_PIN A9 -#endif - -#if !defined(SERIAL_USART_RX_PIN) -# define SERIAL_USART_RX_PIN A10 -#endif - #if !defined(USART_CR1_M0) # define USART_CR1_M0 USART_CR1_M // some platforms (f1xx) dont have this so #endif @@ -89,31 +112,3 @@ typedef SIOConfig QMKSerialConfig; (AFIO->MAPR |= AFIO_MAPR_USART3_REMAP_FULLREMAP); \ } while (0) #endif - -#if !defined(SELECT_SOFT_SERIAL_SPEED) -# define SELECT_SOFT_SERIAL_SPEED 1 -#endif - -#if defined(SERIAL_USART_SPEED) -// Allow advanced users to directly set SERIAL_USART_SPEED -#elif SELECT_SOFT_SERIAL_SPEED == 0 -# define SERIAL_USART_SPEED 460800 -#elif SELECT_SOFT_SERIAL_SPEED == 1 -# define SERIAL_USART_SPEED 230400 -#elif SELECT_SOFT_SERIAL_SPEED == 2 -# define SERIAL_USART_SPEED 115200 -#elif SELECT_SOFT_SERIAL_SPEED == 3 -# define SERIAL_USART_SPEED 57600 -#elif SELECT_SOFT_SERIAL_SPEED == 4 -# define SERIAL_USART_SPEED 38400 -#elif SELECT_SOFT_SERIAL_SPEED == 5 -# define SERIAL_USART_SPEED 19200 -#else -# error invalid SELECT_SOFT_SERIAL_SPEED value -#endif - -#if !defined(SERIAL_USART_TIMEOUT) -# define SERIAL_USART_TIMEOUT 20 -#endif - -#define HANDSHAKE_MAGIC 7U