fix: make uart tx enqueue all-or-nothing

App/uart_trans.c

@@ -32,10 +32,27 @@ typedef struct {
     volatile uint16_t tx_tail;
     volatile uint16_t tx_dma_len;
     volatile uint8_t tx_busy;
+    volatile uint8_t tx_kick_fail_logged;
 } uart_channel_ctx_t;
 
 static uart_channel_ctx_t g_channels[UART_CHANNEL_MAX];
 
+const char *uart_trans_send_result_to_str(uart_trans_send_result_t result)
+{
+    switch (result) {
+    case UART_TRANS_SEND_OK:
+        return "ok";
+    case UART_TRANS_SEND_INVALID_INPUT:
+        return "invalid";
+    case UART_TRANS_SEND_RING_FULL:
+        return "full";
+    case UART_TRANS_SEND_KICK_FAILED:
+        return "kick";
+    default:
+        return "unknown";
+    }
+}
+
 static uint16_t ring_used(uint16_t head, uint16_t tail, uint16_t size)
 {
     return (head >= tail) ? (head - tail) : (size - tail + head);
@@ -66,20 +83,21 @@ static void process_rx_snapshot(uart_channel_t channel)
     }
 }
 
-static void kick_tx(uart_channel_t channel)
+static uart_trans_send_result_t kick_tx(uart_channel_t channel)
 {
     uart_channel_ctx_t *ctx = &g_channels[channel];
     uint16_t available;
     uint16_t chunk;
+    uint16_t tail;
     uint16_t i;
 
     if (ctx->tx_busy != 0u) {
-        return;
+        return UART_TRANS_SEND_OK;
     }
 
     available = ring_used(ctx->tx_head, ctx->tx_tail, UART_TX_RING_BUFFER_SIZE);
     if (available == 0u) {
-        return;
+        return UART_TRANS_SEND_OK;
     }
 
     chunk = available;
@@ -87,16 +105,28 @@ static void kick_tx(uart_channel_t channel)
         chunk = UART_TX_DMA_BUFFER_SIZE;
     }
 
+    tail = ctx->tx_tail;
     for (i = 0; i < chunk; ++i) {
-        ctx->tx_dma_buffer[i] = ctx->tx_ring[ctx->tx_tail];
+        ctx->tx_dma_buffer[i] = ctx->tx_ring[tail];
-        ctx->tx_tail = (uint16_t)((ctx->tx_tail + 1u) % UART_TX_RING_BUFFER_SIZE);
+        tail = (uint16_t)((tail + 1u) % UART_TX_RING_BUFFER_SIZE);
     }
 
+    if (HAL_UART_Transmit_DMA(ctx->huart, ctx->tx_dma_buffer, chunk) != HAL_OK) {
+        ctx->tx_dma_len = 0u;
+        if (ctx->tx_kick_fail_logged == 0u) {
+            debug_log_printf("[UART] kick-fail ch=%u len=%u\r\n",
+                             (unsigned int)channel,
+                             (unsigned int)chunk);
+            ctx->tx_kick_fail_logged = 1u;
+        }
+        return UART_TRANS_SEND_KICK_FAILED;
+    }
+
+    ctx->tx_tail = tail;
     ctx->tx_dma_len = chunk;
     ctx->tx_busy = 1u;
-    if (HAL_UART_Transmit_DMA(ctx->huart, ctx->tx_dma_buffer, chunk) != HAL_OK) {
+    ctx->tx_kick_fail_logged = 0u;
-        ctx->tx_busy = 0u;
+    return UART_TRANS_SEND_OK;
-    }
 }
 
 static uint16_t uart_ring_available(uart_channel_t channel)
@@ -143,6 +173,7 @@ static void uart_route_raw_channel(uart_channel_t channel)
     uint16_t len;
     uint8_t uart_endpoint = (channel == UART_CHANNEL_U1) ? ENDPOINT_UART3 : ENDPOINT_UART2;
     uint32_t i;
+    route_send_result_t route_result;
 
     len = uart_ring_read(channel, buffer, sizeof(buffer));
     if (len == 0u) {
@@ -154,38 +185,143 @@ static void uart_route_raw_channel(uart_channel_t channel)
             continue;
         }
 
-        (void)route_send(xLinkTxQueues[i],
+        route_result = route_send(xLinkTxQueues[i],
                                   uart_endpoint,
                                   config_link_index_to_endpoint((uint8_t)i),
                                   (channel == UART_CHANNEL_U1) ? ROUTE_CONN_UART3 : ROUTE_CONN_UART2,
                                   buffer,
                                   len,
                                   pdMS_TO_TICKS(10));
+        if (route_result != ROUTE_SEND_OK) {
+            debug_log_printf("[UART] raw-route-fail idx=%u rc=%s len=%u\r\n",
+                             (unsigned int)i,
+                             route_send_result_to_str(route_result),
+                             (unsigned int)len);
+        }
     }
 }
 
-static void uart_send_tcp_msg_to_uarts(route_msg_t *msg)
+static uart_trans_send_result_t uart_send_tcp_msg_chunk(route_msg_t *msg,
+                                                        uint16_t offset,
+                                                        uint16_t *accepted_len)
 {
     uint8_t frame[ROUTE_MSG_MAX_PAYLOAD + 6u];
     uint16_t frame_len = 0u;
+    uint16_t remaining;
+    uint16_t chunk_len;
+    uint8_t uart_mask;
+    uart_trans_send_result_t uart_result;
 
-    if ((msg->dst_mask & ENDPOINT_UART2) != 0u) {
+    if (accepted_len == NULL || msg == NULL || offset >= msg->len) {
-        if (config_get()->mux_mode == MUX_MODE_FRAME) {
+        return UART_TRANS_SEND_INVALID_INPUT;
-            if (uart_mux_encode_frame(msg->src_id, ENDPOINT_UART2, msg->data, msg->len, frame, &frame_len, sizeof(frame))) {
-                (void)uart_trans_send_buffer(UART_CHANNEL_U0, frame, frame_len);
-            }
-        } else {
-            (void)uart_trans_send_buffer(UART_CHANNEL_U0, msg->data, msg->len);
-        }
     }
 
-    if ((msg->dst_mask & ENDPOINT_UART3) != 0u) {
+    *accepted_len = 0u;
-        if (config_get()->mux_mode == MUX_MODE_FRAME) {
+
-            if (uart_mux_encode_frame(msg->src_id, ENDPOINT_UART3, msg->data, msg->len, frame, &frame_len, sizeof(frame))) {
+    uart_mask = (uint8_t)(msg->dst_mask & (ENDPOINT_UART2 | ENDPOINT_UART3));
-                (void)uart_trans_send_buffer(UART_CHANNEL_U1, frame, frame_len);
+    if ((msg->dst_mask != uart_mask) ||
+        (uart_mask != ENDPOINT_UART2 && uart_mask != ENDPOINT_UART3)) {
+        return UART_TRANS_SEND_INVALID_INPUT;
     }
-        } else {
+
-            (void)uart_trans_send_buffer(UART_CHANNEL_U1, msg->data, msg->len);
+    remaining = (uint16_t)(msg->len - offset);
+
+    if (uart_mask == ENDPOINT_UART2) {
+        if (config_get()->mux_mode == MUX_MODE_FRAME) {
+            chunk_len = remaining;
+            if (chunk_len > (uint16_t)(UART_TX_RING_BUFFER_SIZE - 1u - 6u)) {
+                chunk_len = (uint16_t)(UART_TX_RING_BUFFER_SIZE - 1u - 6u);
+            }
+            if (!uart_mux_encode_frame(msg->src_id, ENDPOINT_UART2, &msg->data[offset], chunk_len, frame, &frame_len, sizeof(frame))) {
+                return UART_TRANS_SEND_INVALID_INPUT;
+            }
+            uart_result = uart_trans_send_buffer(UART_CHANNEL_U0, frame, frame_len);
+            if (uart_result != UART_TRANS_SEND_OK) {
+                return uart_result;
+            }
+            *accepted_len = chunk_len;
+            return UART_TRANS_SEND_OK;
+        }
+
+        chunk_len = remaining;
+        if (chunk_len > (uint16_t)(UART_TX_RING_BUFFER_SIZE - 1u)) {
+            chunk_len = (uint16_t)(UART_TX_RING_BUFFER_SIZE - 1u);
+        }
+        uart_result = uart_trans_send_buffer(UART_CHANNEL_U0, &msg->data[offset], chunk_len);
+        if (uart_result != UART_TRANS_SEND_OK) {
+            return uart_result;
+        }
+        *accepted_len = chunk_len;
+        return UART_TRANS_SEND_OK;
+    }
+
+    if (config_get()->mux_mode == MUX_MODE_FRAME) {
+        chunk_len = remaining;
+        if (chunk_len > (uint16_t)(UART_TX_RING_BUFFER_SIZE - 1u - 6u)) {
+            chunk_len = (uint16_t)(UART_TX_RING_BUFFER_SIZE - 1u - 6u);
+        }
+        if (!uart_mux_encode_frame(msg->src_id, ENDPOINT_UART3, &msg->data[offset], chunk_len, frame, &frame_len, sizeof(frame))) {
+            return UART_TRANS_SEND_INVALID_INPUT;
+        }
+        uart_result = uart_trans_send_buffer(UART_CHANNEL_U1, frame, frame_len);
+        if (uart_result != UART_TRANS_SEND_OK) {
+            return uart_result;
+        }
+        *accepted_len = chunk_len;
+        return UART_TRANS_SEND_OK;
+    }
+
+    chunk_len = remaining;
+    if (chunk_len > (uint16_t)(UART_TX_RING_BUFFER_SIZE - 1u)) {
+        chunk_len = (uint16_t)(UART_TX_RING_BUFFER_SIZE - 1u);
+    }
+    uart_result = uart_trans_send_buffer(UART_CHANNEL_U1, &msg->data[offset], chunk_len);
+    if (uart_result != UART_TRANS_SEND_OK) {
+        return uart_result;
+    }
+    *accepted_len = chunk_len;
+    return UART_TRANS_SEND_OK;
+}
+
+static void uart_try_advance_pending_tcp_msg(route_msg_t **pending_tcp_msg,
+                                             uint16_t *pending_tcp_offset,
+                                             uart_trans_send_result_t *pending_tcp_result)
+{
+    route_msg_t *msg;
+    uart_trans_send_result_t uart_result;
+    uint16_t accepted_len;
+
+    if (pending_tcp_msg == NULL || pending_tcp_offset == NULL || pending_tcp_result == NULL) {
+        return;
+    }
+
+    msg = *pending_tcp_msg;
+    if (msg == NULL) {
+        return;
+    }
+
+    for (;;) {
+        accepted_len = 0u;
+        uart_result = uart_send_tcp_msg_chunk(msg, *pending_tcp_offset, &accepted_len);
+        if (uart_result != UART_TRANS_SEND_OK) {
+            if (uart_result != *pending_tcp_result) {
+                debug_log_printf("[UART] tcp-pend src=0x%02X dst=0x%02X off=%u rc=%s\r\n",
+                                 (unsigned int)msg->src_id,
+                                 (unsigned int)msg->dst_mask,
+                                 (unsigned int)(*pending_tcp_offset),
+                                 uart_trans_send_result_to_str(uart_result));
+                *pending_tcp_result = uart_result;
+            }
+            break;
+        }
+
+        *pending_tcp_offset = (uint16_t)(*pending_tcp_offset + accepted_len);
+        *pending_tcp_result = UART_TRANS_SEND_OK;
+        if (*pending_tcp_offset >= msg->len) {
+            route_msg_free(msg);
+            *pending_tcp_msg = NULL;
+            *pending_tcp_offset = 0u;
+            break;
         }
     }
 }
@@ -194,18 +330,26 @@ static void uart_route_mux_frame(uart_channel_t source_channel, const uart_mux_f
 {
     const device_config_t *cfg = config_get();
     uint32_t i;
+    uint8_t endpoint;
     uint8_t source_conn = (source_channel == UART_CHANNEL_U1) ? ROUTE_CONN_UART3 : ROUTE_CONN_UART2;
     uint8_t out_frame[ROUTE_MSG_MAX_PAYLOAD + 6u];
     uint16_t out_frame_len = 0u;
+    route_send_result_t route_result;
+    uart_trans_send_result_t uart_result;
 
     if (frame->dst_mask == 0u) {
-        (void)route_send(xConfigQueue,
+        route_result = route_send(xConfigQueue,
                                   frame->src_id,
                                   0u,
                                   source_conn,
                                   frame->payload,
                                   frame->payload_len,
                                   pdMS_TO_TICKS(10));
+        if (route_result != ROUTE_SEND_OK) {
+            debug_log_printf("[UART] mux-cfg-fail rc=%s len=%u\r\n",
+                             route_send_result_to_str(route_result),
+                             (unsigned int)frame->payload_len);
+        }
         return;
     }
 
@@ -213,21 +357,41 @@ static void uart_route_mux_frame(uart_channel_t source_channel, const uart_mux_f
         if (cfg->links[i].enabled == 0u) {
             continue;
         }
-        uint8_t endpoint = config_link_index_to_endpoint((uint8_t)i);
+        endpoint = config_link_index_to_endpoint((uint8_t)i);
         if ((frame->dst_mask & endpoint) != 0u) {
-            (void)route_send(xLinkTxQueues[i], frame->src_id, endpoint, source_conn, frame->payload, frame->payload_len, pdMS_TO_TICKS(10));
+            route_result = route_send(xLinkTxQueues[i], frame->src_id, endpoint, source_conn, frame->payload, frame->payload_len, pdMS_TO_TICKS(10));
+            if (route_result != ROUTE_SEND_OK) {
+                debug_log_printf("[UART] mux-route-fail idx=%u rc=%s len=%u\r\n",
+                                 (unsigned int)i,
+                                 route_send_result_to_str(route_result),
+                                 (unsigned int)frame->payload_len);
+            }
         }
     }
 
     if ((frame->dst_mask & ENDPOINT_UART2) != 0u && source_channel != UART_CHANNEL_U0) {
         if (uart_mux_encode_frame(frame->src_id, ENDPOINT_UART2, frame->payload, frame->payload_len, out_frame, &out_frame_len, sizeof(out_frame))) {
-            (void)uart_trans_send_buffer(UART_CHANNEL_U0, out_frame, out_frame_len);
+            uart_result = uart_trans_send_buffer(UART_CHANNEL_U0, out_frame, out_frame_len);
+            if (uart_result != UART_TRANS_SEND_OK) {
+                debug_log_printf("[UART] mux-u0-tx-fail rc=%s len=%u\r\n",
+                                 uart_trans_send_result_to_str(uart_result),
+                                 (unsigned int)out_frame_len);
+            }
+        } else {
+            debug_log_printf("[UART] mux-u0-enc-fail len=%u\r\n", (unsigned int)frame->payload_len);
         }
     }
 
     if ((frame->dst_mask & ENDPOINT_UART3) != 0u && source_channel != UART_CHANNEL_U1) {
         if (uart_mux_encode_frame(frame->src_id, ENDPOINT_UART3, frame->payload, frame->payload_len, out_frame, &out_frame_len, sizeof(out_frame))) {
-            (void)uart_trans_send_buffer(UART_CHANNEL_U1, out_frame, out_frame_len);
+            uart_result = uart_trans_send_buffer(UART_CHANNEL_U1, out_frame, out_frame_len);
+            if (uart_result != UART_TRANS_SEND_OK) {
+                debug_log_printf("[UART] mux-u1-tx-fail rc=%s len=%u\r\n",
+                                 uart_trans_send_result_to_str(uart_result),
+                                 (unsigned int)out_frame_len);
+            }
+        } else {
+            debug_log_printf("[UART] mux-u1-enc-fail len=%u\r\n", (unsigned int)frame->payload_len);
         }
     }
 }
@@ -273,22 +437,44 @@ int uart_trans_start_all(void)
     return 0;
 }
 
-bool uart_trans_send_buffer(uart_channel_t channel, const uint8_t *data, uint16_t len)
+uart_trans_send_result_t uart_trans_send_buffer(uart_channel_t channel, const uint8_t *data, uint16_t len)
 {
-    uart_channel_ctx_t *ctx = &g_channels[channel];
+    uart_channel_ctx_t *ctx;
+    uart_trans_send_result_t uart_result;
+    uint16_t original_head;
     uint16_t written = 0u;
 
-    if (data == NULL || len == 0u) {
+    if (channel >= UART_CHANNEL_MAX || data == NULL || len == 0u || len >= UART_TX_RING_BUFFER_SIZE) {
-        return false;
+        return UART_TRANS_SEND_INVALID_INPUT;
     }
 
-    while (written < len && ring_free(ctx->tx_head, ctx->tx_tail, UART_TX_RING_BUFFER_SIZE) > 0u) {
+    ctx = &g_channels[channel];
+    if (ctx->huart == NULL) {
+        return UART_TRANS_SEND_INVALID_INPUT;
+    }
+
+    taskENTER_CRITICAL();
+    original_head = ctx->tx_head;
+    if (ring_free(ctx->tx_head, ctx->tx_tail, UART_TX_RING_BUFFER_SIZE) < len) {
+        taskEXIT_CRITICAL();
+        return UART_TRANS_SEND_RING_FULL;
+    }
+
+    while (written < len) {
         ctx->tx_ring[ctx->tx_head] = data[written++];
         ctx->tx_head = (uint16_t)((ctx->tx_head + 1u) % UART_TX_RING_BUFFER_SIZE);
     }
 
-    kick_tx(channel);
+    uart_result = kick_tx(channel);
-    return (written == len);
+    if (uart_result != UART_TRANS_SEND_OK) {
+        ctx->tx_head = original_head;
+        taskEXIT_CRITICAL();
+        return uart_result;
+    }
+
+    taskEXIT_CRITICAL();
+
+    return UART_TRANS_SEND_OK;
 }
 
 void uart_trans_notify_rx_from_isr(uart_channel_t channel, BaseType_t *xHigherPriorityTaskWoken)
@@ -412,8 +598,11 @@ void UartRxTask(void *argument)
     uint32_t notify_value;
     BaseType_t notified;
     route_msg_t *msg;
+    route_msg_t *pending_tcp_msg = NULL;
+    uint16_t pending_tcp_offset = 0u;
     uart_mux_frame_t frame;
     const device_config_t *cfg;
+    uart_trans_send_result_t pending_tcp_result = UART_TRANS_SEND_OK;
 
     (void)argument;
     if (uart_trans_start_all() != 0) {
@@ -439,9 +628,13 @@ void UartRxTask(void *argument)
             g_channels[UART_CHANNEL_U1].tx_busy = 0u;
         }
 
-        while (xQueueReceive(xTcpRxQueue, &msg, 0) == pdPASS) {
+        uart_try_advance_pending_tcp_msg(&pending_tcp_msg, &pending_tcp_offset, &pending_tcp_result);
-            uart_send_tcp_msg_to_uarts(msg);
+
-            route_msg_free(msg);
+        while (pending_tcp_msg == NULL && xQueueReceive(xTcpRxQueue, &msg, 0) == pdPASS) {
+            pending_tcp_msg = msg;
+            pending_tcp_offset = 0u;
+            pending_tcp_result = UART_TRANS_SEND_OK;
+            uart_try_advance_pending_tcp_msg(&pending_tcp_msg, &pending_tcp_offset, &pending_tcp_result);
         }
 
         cfg = config_get();

App/uart_trans.h

@@ -16,6 +16,13 @@ typedef enum {
     UART_CHANNEL_MAX
 } uart_channel_t;
 
+typedef enum {
+    UART_TRANS_SEND_OK = 0,
+    UART_TRANS_SEND_INVALID_INPUT,
+    UART_TRANS_SEND_RING_FULL,
+    UART_TRANS_SEND_KICK_FAILED
+} uart_trans_send_result_t;
+
 typedef struct {
     uint8_t src_id;
     uint8_t dst_mask;
@@ -31,7 +38,8 @@ typedef struct {
 int uart_trans_init(void);
 int uart_trans_config(uint8_t uart_index, uint32_t baudrate);
 int uart_trans_start_all(void);
-bool uart_trans_send_buffer(uart_channel_t channel, const uint8_t *data, uint16_t len);
+const char *uart_trans_send_result_to_str(uart_trans_send_result_t result);
+uart_trans_send_result_t uart_trans_send_buffer(uart_channel_t channel, const uint8_t *data, uint16_t len);
 void uart_trans_notify_rx_from_isr(uart_channel_t channel, BaseType_t *xHigherPriorityTaskWoken);
 void uart_trans_tx_cplt_handler(uart_channel_t channel);
 void UartRxTask(void *argument);