nrf_rpc_uart: decode ACKs in ISR

Currently, decoding both normal packets and ACKs happens
in the receive thread. This imposes certain limitations,
for example the receive thread shall not send any packets
as this would introduce a chicken-egg problem: the thread
is waiting for an ACK but the ACK can only be decoded by
the current thread.

More importantly, this can also introduce spurious ACK
timeouts and retransissions because the receive thread may
be blocked when trying to push a packet to the thead pool.

Move the ACK decoding to ISR to overcome the mentioned
issues.

Author: Damian-Nordic

subsys/nrf_rpc/nrf_rpc_uart.c

@@ -60,6 +60,11 @@ struct nrf_rpc_uart {
 
 	K_KERNEL_STACK_MEMBER(rx_workq_stack, CONFIG_NRF_RPC_UART_RX_THREAD_STACK_SIZE);
 
+	/* HDLC ack decoding state */
+	enum hdlc_state rx_ack_state;
+	uint8_t rx_ack[2];
+	size_t rx_ack_len;
+
 	/* HDLC packet decoding state */
 	enum hdlc_state rx_pkt_state;
 	size_t rx_pkt_len;
@@ -95,12 +100,12 @@ static void send_byte(const struct device *dev, uint8_t byte);
 
 static void ack_rx(struct nrf_rpc_uart *uart_tr)
 {
-	if (!IS_ENABLED(CONFIG_NRF_RPC_UART_RELIABLE) || uart_tr->rx_pkt_len != CRC_SIZE) {
-		log_hexdump_dbg(uart_tr->rx_pkt, uart_tr->rx_pkt_len, ">>> RX invalid frame");
+	if (!IS_ENABLED(CONFIG_NRF_RPC_UART_RELIABLE) || uart_tr->rx_ack_len != CRC_SIZE) {
+		log_hexdump_dbg(uart_tr->rx_ack, uart_tr->rx_ack_len, ">>> RX invalid frame");
 		return;
 	}
 
-	uint16_t rx_ack = sys_get_le16(uart_tr->rx_pkt);
+	uint16_t rx_ack = sys_get_le16(uart_tr->rx_ack);
 
 	LOG_DBG(">>> RX ack %04x", rx_ack);
 
@@ -110,7 +115,6 @@ static void ack_rx(struct nrf_rpc_uart *uart_tr)
 	}
 
 	k_sem_give(&uart_tr->ack_sem);
-	k_yield();
 }
 
 static void ack_tx(struct nrf_rpc_uart *uart_tr, uint16_t ack_pld)
@@ -241,7 +245,7 @@ static void work_handler(struct k_work *work)
 			}
 
 			if (uart_tr->rx_pkt_len <= CRC_SIZE) {
-				ack_rx(uart_tr);
+				/* ACKs are already handled in ISR, so only process normal packets in this handler */
 				continue;
 			}
 
@@ -276,6 +280,18 @@ static void work_handler(struct k_work *work)
 	}
 }
 
+static void decode_ack(struct nrf_rpc_uart *inst, const uint8_t *rx, size_t len)
+{
+	for (size_t i = 0; i < len; i++) {
+		hdlc_decode_byte(&inst->rx_ack_state, inst->rx_ack, &inst->rx_ack_len,
+				 sizeof(inst->rx_ack), rx[i]);
+
+		if (inst->rx_ack_state == HDLC_STATE_FRAME_FOUND) {
+			ack_rx(inst);
+		}
+	}
+}
+
 static void serial_cb(const struct device *uart, void *user_data)
 {
 	struct nrf_rpc_uart *uart_tr = user_data;
@@ -288,6 +304,7 @@ static void serial_cb(const struct device *uart, void *user_data)
 					    uart_tr->rx_ringbuf.size);
 		if (rx_len > 0) {
 			rx_len = uart_fifo_read(uart, rx_buffer, rx_len);
+			decode_ack(uart_tr, rx_buffer, rx_len);
 			int err = ring_buf_put_finish(&uart_tr->rx_ringbuf, rx_len);
 			(void)err; /*silence the compiler*/
 			__ASSERT_NO_MSG(err == 0);
@@ -365,6 +382,7 @@ static int init(const struct nrf_rpc_tr *transport, nrf_rpc_tr_receive_handler_t
 	k_work_init(&uart_tr->rx_work, work_handler);
 	ring_buf_init(&uart_tr->rx_ringbuf, sizeof(uart_tr->rx_buffer), uart_tr->rx_buffer);
 
+	uart_tr->rx_ack_state = HDLC_STATE_UNSYNC;
 	uart_tr->rx_pkt_state = HDLC_STATE_UNSYNC;
 	uart_irq_rx_enable(uart_tr->uart);