Update TX to async and fix RX thread support

Refactor the code to common code

app/CMakeLists.txt

@@ -78,9 +78,11 @@ endif()
 if (CONFIG_ZMK_SPLIT_SERIAL AND (NOT CONFIG_ZMK_SPLIT_SERIAL_ROLE_CENTRAL))
 	target_sources(app PRIVATE src/split_listener.c)
 	target_sources(app PRIVATE src/split/serial/service.c)
+	target_sources(app PRIVATE src/split/serial/common.c)
 endif()
 if (CONFIG_ZMK_SPLIT_SERIAL AND CONFIG_ZMK_SPLIT_SERIAL_ROLE_CENTRAL)
 	target_sources(app PRIVATE src/split/serial/central.c)
+	target_sources(app PRIVATE src/split/serial/common.c)
 endif()
 target_sources_ifdef(CONFIG_USB app PRIVATE src/usb.c)
 target_sources_ifdef(CONFIG_ZMK_BLE app PRIVATE src/hog.c)

app/Kconfig

@@ -243,7 +243,7 @@ endchoice
 
 config ZMK_SPLIT_SERIAL_THREAD_STACK_SIZE
 	int "Serial split thread stack size"
-	default 128
+	default 1024
 
 config ZMK_SPLIT_SERIAL_THREAD_PRIORITY
 	int "Serial split thread priority"

app/include/zmk/split/serial/common.h

@@ -0,0 +1,20 @@
+/*
+ * Copyright (c) 2022 The ZMK Contributors
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#pragma once
+
+#include <zephyr/types.h>
+#include <init.h>
+
+typedef int (*rx_complete_t)(const uint8_t *data, size_t length);
+
+void split_serial_async_init(rx_complete_t complete_fn);
+
+void split_serial_async_send(uint8_t *data, size_t length);
+
+uint8_t *alloc_position_state_buffer(k_timeout_t timeout);
+
+void free_position_state_buffer(const uint8_t *data);

app/src/split/serial/central.c

@@ -5,6 +5,7 @@
  */
 
 #include <zmk/split/common.h>
+#include <zmk/split/serial/common.h>
 #include <init.h>
 
 #include <sys/crc.h>
@@ -19,26 +20,9 @@ LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);
 #include <zmk/events/position_state_changed.h>
 #include <zmk/matrix.h>
 
-#if !DT_HAS_CHOSEN(zmk_split_serial)
-#error "No zmk-split-serial node is chosen"
-#endif
-
-#define UART_NODE1 DT_CHOSEN(zmk_split_serial)
-const struct device *serial_dev = DEVICE_DT_GET(UART_NODE1);
-static int uart_ready = 0;
-
-static void split_serial_receive_thread(void *unused, void *unused1, void *unused2);
-
-K_MEM_SLAB_DEFINE(split_memory_slab, sizeof(split_data_t),
-                  CONFIG_ZMK_SPLIT_SERIAL_THREAD_QUEUE_SIZE, 4);
-
 K_MSGQ_DEFINE(peripheral_event_msgq, sizeof(struct zmk_position_state_changed),
               CONFIG_ZMK_SPLIT_SERIAL_THREAD_QUEUE_SIZE, 4);
 
-K_THREAD_DEFINE(split_central, CONFIG_ZMK_SPLIT_SERIAL_THREAD_STACK_SIZE,
-                split_serial_receive_thread, NULL, NULL, NULL,
-                K_PRIO_PREEMPT(CONFIG_ZMK_SPLIT_SERIAL_THREAD_PRIORITY), 0, 0);
-
 static void peripheral_event_work_callback(struct k_work *work) {
     struct zmk_position_state_changed ev;
     while (k_msgq_get(&peripheral_event_msgq, &ev, K_NO_WAIT) == 0) {
@@ -49,10 +33,10 @@ static void peripheral_event_work_callback(struct k_work *work) {
 
 K_WORK_DEFINE(peripheral_event_work, peripheral_event_work_callback);
 
-static uint8_t split_central_notify_func(const void *data, uint16_t length) {
+static int split_central_notify_func(const uint8_t *data, size_t length) {
     static uint8_t position_state[SPLIT_DATA_LEN];
     uint8_t changed_positions[SPLIT_DATA_LEN];
-    const split_data_t *split_data = data;
+    const split_data_t *split_data = (const split_data_t *)data;
     uint16_t crc;
 
     LOG_DBG("[NOTIFICATION] data %p type:%u CRC:%u", data, split_data->type, split_data->crc);
@@ -90,90 +74,9 @@ static uint8_t split_central_notify_func(const void *data, uint16_t length) {
     return 0;
 }
 
-static char *alloc_position_state_buffer() {
+static int split_serial_central_init(const struct device *dev) {
-    char *block_ptr = NULL;
+    split_serial_async_init(split_central_notify_func);
-    if (k_mem_slab_alloc(&split_memory_slab, (void **)&block_ptr, K_NO_WAIT) == 0) {
+    return 0;
-        memset(block_ptr, 0, SPLIT_DATA_LEN);
-    } else {
-        LOG_WRN("Memory allocation time-out");
-    }
-    return block_ptr;
 }
 
-static void free_position_state_buffer(char *block_ptr) {
+SYS_INIT(split_serial_central_init, APPLICATION, CONFIG_ZMK_USB_INIT_PRIORITY);
-    k_mem_slab_free(&split_memory_slab, (void **)&block_ptr);
-}
-
-static void uart_callback(const struct device *dev, struct uart_event *evt, void *user_data) {
-    char *buf = NULL;
-
-    switch (evt->type) {
-
-    case UART_RX_STOPPED:
-        LOG_DBG("UART device:%s rx stopped", serial_dev->name);
-        break;
-
-    case UART_RX_BUF_REQUEST:
-        LOG_DBG("UART device:%s rx extra buf req", serial_dev->name);
-        buf = alloc_position_state_buffer();
-        if (NULL != buf) {
-            int ret = uart_rx_buf_rsp(serial_dev, buf, sizeof(split_data_t));
-            if (0 != ret) {
-                LOG_WRN("UART device:%s rx extra buf req add failed: %d", serial_dev->name, ret);
-                free_position_state_buffer(buf);
-            }
-        }
-        break;
-
-    case UART_RX_RDY:
-        LOG_DBG("UART device:%s rx buf ready", serial_dev->name);
-        break;
-
-    case UART_RX_BUF_RELEASED:
-        LOG_DBG("UART device:%s rx buf released", serial_dev->name);
-        split_central_notify_func(evt->data.rx_buf.buf, sizeof(split_data_t));
-        free_position_state_buffer(evt->data.rx_buf.buf);
-        break;
-
-    default:
-        LOG_DBG("UART device:%s unhandled event: %u", serial_dev->name, evt->type);
-        break;
-    };
-    return;
-}
-
-static void split_serial_receive_thread(void *unused, void *unused1, void *unused2) {
-    if (!device_is_ready(serial_dev)) {
-        LOG_WRN("UART device:%s not ready", serial_dev->name);
-        return;
-    }
-
-    int ret = uart_callback_set(serial_dev, uart_callback, NULL);
-    if (ret == -ENOTSUP || ret == -ENOSYS) {
-        LOG_WRN("UART device:%s ASYNC not supported", serial_dev->name);
-        return;
-    }
-
-    uart_ready = 1;
-    LOG_DBG("UART device:%s ready", serial_dev->name);
-
-    while (true) {
-        char *buf = alloc_position_state_buffer();
-        if (NULL == buf) {
-            k_msleep(100);
-            continue;
-        }
-
-        int ret = uart_rx_enable(serial_dev, buf, sizeof(split_data_t), SYS_FOREVER_MS);
-        if (ret == -ENOTSUP) {
-            LOG_WRN("UART device:%s not supporting DMA", serial_dev->name);
-            free_position_state_buffer(buf);
-            return;
-        }
-        if (ret != 0 && ret != -EBUSY) {
-            LOG_WRN("UART device:%s RX error:%d", serial_dev->name, ret);
-            free_position_state_buffer(buf);
-            continue;
-        }
-    };
-}

app/src/split/serial/common.c

@@ -0,0 +1,153 @@
+/*
+ * Copyright (c) 2022 The ZMK Contributors
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#include <zmk/split/common.h>
+#include <zmk/split/serial/common.h>
+#include <init.h>
+
+#include <sys/crc.h>
+#include <device.h>
+#include <drivers/uart.h>
+
+#include <logging/log.h>
+
+LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);
+
+#include <zmk/event_manager.h>
+#include <zmk/events/position_state_changed.h>
+#include <zmk/matrix.h>
+
+#if !DT_HAS_CHOSEN(zmk_split_serial)
+#error "No zmk-split-serial node is chosen"
+#endif
+
+#define UART_NODE1 DT_CHOSEN(zmk_split_serial)
+const struct device *serial_dev = DEVICE_DT_GET(UART_NODE1);
+static int uart_ready = 0;
+
+K_MEM_SLAB_DEFINE(split_memory_slab, sizeof(split_data_t),
+                  CONFIG_ZMK_SPLIT_SERIAL_THREAD_QUEUE_SIZE, 4);
+
+static K_SEM_DEFINE(split_serial_rx_sem, 1, 1);
+
+static K_SEM_DEFINE(split_serial_tx_sem, 1, 1);
+
+rx_complete_t split_serial_rx_complete_fn = NULL;
+
+uint8_t *alloc_position_state_buffer(k_timeout_t timeout) {
+    uint8_t *block_ptr = NULL;
+    if (k_mem_slab_alloc(&split_memory_slab, (void **)&block_ptr, timeout) == 0) {
+        memset(block_ptr, 0, SPLIT_DATA_LEN);
+    } else {
+        LOG_WRN("Memory allocation time-out");
+    }
+    return block_ptr;
+}
+
+void free_position_state_buffer(const uint8_t *data) {
+    k_mem_slab_free(&split_memory_slab, (void **)&data);
+}
+
+static void enable_rx(const struct device *dev) {
+    int ret;
+    uint8_t *buf = NULL;
+    while (!(buf = alloc_position_state_buffer(K_MSEC(100)))) {
+    };
+
+    while (0 != (ret = uart_rx_enable(serial_dev, buf, sizeof(split_data_t), SYS_FOREVER_MS))) {
+        LOG_WRN("UART device:%s RX error:%d", serial_dev->name, ret);
+        k_sleep(K_MSEC(100));
+    }
+    return;
+}
+
+static void uart_callback(const struct device *dev, struct uart_event *evt, void *user_data) {
+    uint8_t *buf = NULL;
+
+    switch (evt->type) {
+
+    case UART_RX_STOPPED:
+        LOG_DBG("UART device:%s rx stopped", serial_dev->name);
+        break;
+
+    case UART_RX_BUF_REQUEST:
+        LOG_DBG("UART device:%s rx extra buf req", serial_dev->name);
+        buf = alloc_position_state_buffer(K_NO_WAIT);
+        if (NULL != buf) {
+            int ret = uart_rx_buf_rsp(serial_dev, buf, sizeof(split_data_t));
+            if (0 != ret) {
+                LOG_WRN("UART device:%s rx extra buf req add failed: %d", serial_dev->name, ret);
+                free_position_state_buffer(buf);
+            }
+        }
+        break;
+
+    case UART_RX_RDY:
+        LOG_DBG("UART device:%s rx buf ready", serial_dev->name);
+        break;
+
+    case UART_RX_BUF_RELEASED:
+        LOG_DBG("UART device:%s rx buf released", serial_dev->name);
+        if (split_serial_rx_complete_fn) {
+            split_serial_rx_complete_fn(evt->data.rx_buf.buf, sizeof(split_data_t));
+        }
+        free_position_state_buffer(evt->data.rx_buf.buf);
+        break;
+
+    case UART_RX_DISABLED:
+        LOG_WRN("UART device:%s rx disabled", serial_dev->name);
+        enable_rx(serial_dev);
+        break;
+
+    case UART_TX_DONE:
+        LOG_DBG("UART device:%s tx done", serial_dev->name);
+        free_position_state_buffer(evt->data.tx.buf);
+        k_sem_give(&split_serial_tx_sem);
+        break;
+
+    case UART_TX_ABORTED:
+        LOG_WRN("UART device:%s tx aborted", serial_dev->name);
+        k_sem_give(&split_serial_tx_sem);
+        break;
+
+    default:
+        LOG_DBG("UART device:%s unhandled event: %u", serial_dev->name, evt->type);
+        break;
+    };
+    return;
+}
+
+void split_serial_async_send(uint8_t *data, size_t len) {
+    if (!uart_ready) {
+        return;
+    }
+
+    k_sem_take(&split_serial_tx_sem, K_FOREVER);
+    int err = uart_tx(serial_dev, data, len, 0);
+    if (0 != err) {
+        LOG_WRN("Failed to send data via UART: (%d)", err);
+    }
+}
+
+void split_serial_async_init(rx_complete_t rx_comp_fn) {
+    if (!device_is_ready(serial_dev)) {
+        LOG_ERR("UART device:%s not ready", serial_dev->name);
+        return;
+    }
+
+    int ret = uart_callback_set(serial_dev, uart_callback, NULL);
+    if (ret == -ENOTSUP || ret == -ENOSYS) {
+        LOG_WRN("UART device:%s ASYNC not supported", serial_dev->name);
+        return;
+    }
+
+    split_serial_rx_complete_fn = rx_comp_fn;
+
+    uart_ready = 1;
+    LOG_ERR("UART device:%s ready", serial_dev->name);
+
+    enable_rx(serial_dev);
+}

app/src/split/serial/service.c

@@ -5,6 +5,7 @@
  */
 
 #include <zmk/split/common.h>
+#include <zmk/split/serial/common.h>
 #include <sys/util.h>
 #include <sys/crc.h>
 #include <init.h>
@@ -18,14 +19,6 @@ LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);
 
 #include <zmk/matrix.h>
 
-#if !DT_HAS_CHOSEN(zmk_split_serial)
-#error "No zmk-split-serial node is chosen"
-#endif
-
-#define UART_NODE1 DT_CHOSEN(zmk_split_serial)
-const struct device *serial_dev = DEVICE_DT_GET(UART_NODE1);
-static int uart_ready = 0;
-
 static uint8_t position_state[SPLIT_DATA_LEN];
 
 K_THREAD_STACK_DEFINE(service_q_stack, CONFIG_ZMK_SPLIT_SERIAL_THREAD_STACK_SIZE);
@@ -35,28 +28,24 @@ struct k_work_q service_work_q;
 K_MSGQ_DEFINE(position_state_msgq, sizeof(char[SPLIT_DATA_LEN]),
               CONFIG_ZMK_SPLIT_SERIAL_THREAD_QUEUE_SIZE, 4);
 
-void send_data_via_uart(const struct device *dev, char *data, size_t len) {
+static void send_position_state_callback(struct k_work *work) {
-    if (!uart_ready) {
+    split_data_t *split_data = NULL;
-        return;
-    }
 
-    for (int i = 0; i < len; i++) {
+    while (!(split_data = (split_data_t *)alloc_position_state_buffer(K_MSEC(100)))) {
-        uart_poll_out(serial_dev, data[i]);
+    };
-    }
-}
 
-void send_position_state_callback(struct k_work *work) {
+    memset(split_data, sizeof(split_data_t), 0);
-    split_data_t split_data = {.type = SPLIT_TYPE_KEYPOSITION};
+    split_data->type = SPLIT_TYPE_KEYPOSITION;
 
-    while (k_msgq_get(&position_state_msgq, &split_data.data, K_NO_WAIT) == 0) {
+    while (k_msgq_get(&position_state_msgq, split_data->data, K_NO_WAIT) == 0) {
-        split_data.crc = crc16_ansi(split_data.data, sizeof(split_data.data));
+        split_data->crc = crc16_ansi(split_data->data, sizeof(split_data->data));
-        send_data_via_uart(serial_dev, (void *)&split_data, sizeof(split_data));
+        split_serial_async_send((uint8_t *)split_data, sizeof(*split_data));
     }
 };
 
 K_WORK_DEFINE(service_position_notify_work, send_position_state_callback);
 
-int send_position_state() {
+static int send_position_state() {
     int err = k_msgq_put(&position_state_msgq, position_state, K_MSEC(100));
     if (err) {
         switch (err) {
@@ -86,19 +75,12 @@ int zmk_split_position_released(uint8_t position) {
     return send_position_state();
 }
 
-int service_init(const struct device *_arg) {
+static int split_serial_service_init(const struct device *dev) {
-
+    split_serial_async_init(NULL);
-    if (!device_is_ready(serial_dev)) {
-        LOG_WRN("UART device:%s not ready", serial_dev->name);
-        return 1;
-    }
-
-    uart_ready = 1;
-    LOG_INF("UART device:%s ready", serial_dev->name);
     k_work_q_start(&service_work_q, service_q_stack, K_THREAD_STACK_SIZEOF(service_q_stack),
                    CONFIG_ZMK_SPLIT_SERIAL_THREAD_PRIORITY);
 
     return 0;
 }
 
-SYS_INIT(service_init, APPLICATION, CONFIG_ZMK_USB_INIT_PRIORITY);
+SYS_INIT(split_serial_service_init, APPLICATION, CONFIG_ZMK_USB_INIT_PRIORITY);