Improved the comm

prj.conf

@@ -11,23 +11,20 @@ CONFIG_UART_LINE_CTRL=y
 # USB
 CONFIG_USB_DEVICE_STACK_NEXT=y
 CONFIG_CDC_ACM_SERIAL_INITIALIZE_AT_BOOT=n
-# CONFIG_CDC_ACM_SERIAL_PRODUCT_STRING="USB CDC ACM"
-# CONFIG_CDC_ACM_SERIAL_PID=
 CONFIG_USBD_VID=0xffff
 CONFIG_USBD_PID=0x0420
 CONFIG_USBD_MANUFACTURER="Zephyr Project"
 CONFIG_USBD_PRODUCT="USBD sample"
 CONFIG_USBD_SELF_POWERED=y
-# CONFIG_USBD_REMOTE_WAKEUP=
 CONFIG_USBD_MAX_POWER=125
 
 # LOG
-CONFIG_LOG=y
+CONFIG_LOG=n
 CONFIG_USBD_CDC_ACM_LOG_LEVEL_OFF=y  # This removes a pointless warning
 CONFIG_LOG_DEFAULT_LEVEL=3
 CONFIG_LOG_MODE_IMMEDIATE=y
-# CONFIG_USBD_LOG_LEVEL_ERR=y
-# CONFIG_UDC_DRIVER_LOG_LEVEL_ERR=y
 
 # DEBUG
-CONFIG_DEBUG_THREAD_INFO=y
+CONFIG_DEBUG_THREAD_INFO=y
+# CONFIG_DEBUG=y
+# CONFIG_DEBUG_OPTIMIZATIONS=y

scripts/led_blink.py

@@ -10,10 +10,9 @@ BAUDRATE = 115200
 
 COMMAND_PREFIX = 0x69
 
-COMMAND_ERROR = 0
+COMMAND_ACK = 0
-COMMAND_ACK = 1
+COMMAND_NACK = 1
-COMMAND_NACK = 2
+LED = 2
-LED = 3
 
 DEVICE_ID = 0
 

src/command_handler.c

@@ -14,11 +14,6 @@ int command_handler(struct command_message_t *msg) {
 	LOG_DBG("Processing command: %d, length: %d", msg->command, msg->length);
 
 	switch (msg->command) {
-		case COMMAND_ERROR: {
-			LOG_WRN("Received COMMAND_ERROR");
-			break;
-		}
-
 		case LED: {
 			// Toggle LED with the LED command
 			led_toggle();

src/command_message.h

@@ -7,9 +7,9 @@
 
 #define COMMAND_PREFIX 0x69
 #define COMMAND_ID 0x00
+#define COMMAND_DATA_SIZE 160
 
 typedef enum {
-	COMMAND_ERROR,
 	COMMAND_ACK,
 	COMMAND_NACK,
 	LED,
@@ -21,7 +21,7 @@ struct command_message_t {
 	uint8_t id;
 	uint8_t command;
 	uint8_t crc;
-	uint8_t data[160];
+	uint8_t data[COMMAND_DATA_SIZE];
 } __attribute__((packed));
 
 /**

src/usb.c

@@ -6,8 +6,9 @@
 #include <zephyr/device.h>
 #include <zephyr/drivers/uart.h>
 #include <zephyr/kernel.h>
+#include <zephyr/sys/ring_buffer.h>
 
-LOG_MODULE_REGISTER(usb, LOG_LEVEL_DBG);
+LOG_MODULE_REGISTER(usb, LOG_LEVEL_INF);
 
 // DEVICE
 const struct device *const uart_dev = DEVICE_DT_GET_ONE(zephyr_cdc_acm_uart);
@@ -20,15 +21,15 @@ static k_tid_t usb_thread_id = NULL;
 K_THREAD_STACK_DEFINE(usb_thread_stack, USB_THREAD_STACK_SIZE);
 
 // RX BUFFER
-#define RX_BUF_SIZE 4
+#define RING_BUF_SIZE 255
-struct rx_buffer_t {
+static uint8_t ring_buffer[RING_BUF_SIZE];
-	int current_read;
+static struct ring_buf ringbuf;
-	int current_write;
+struct k_sem rx_semaphore;
-	struct k_sem semaphore;
-	struct command_message_t buffer[RX_BUF_SIZE];
-};
 
-static struct rx_buffer_t rx_buf;
+// ACK / NACK messages
+#define RETURN_ACK true
+struct command_message_t ack_msg;
+struct command_message_t nack_msg;
 
 
 static void interrupt_handler(const struct device *dev, void *user_data) {
@@ -42,26 +43,29 @@ static void interrupt_handler(const struct device *dev, void *user_data) {
 		}
 
 		if (uart_irq_rx_ready(dev)) {
-			size_t len = sizeof(struct command_message_t);
+			int recv_len, rb_len;
-			
+			uint8_t buffer[64];
-			int recv_len = uart_fifo_read(dev, (uint8_t *)&rx_buf.buffer[rx_buf.current_write], len);
+			size_t len = MIN(ring_buf_space_get(&ringbuf), sizeof(buffer));
+
+			if (len == 0) {
+				// ring buffer full, drops package(s)
+			    uart_irq_rx_disable(dev);
+				k_sem_give(&rx_semaphore);
+				break;
+			}
+
+			recv_len = uart_fifo_read(dev, buffer, len);
 			if (recv_len < 0) {
 				LOG_ERR("Failed to read UART FIFO");
 				recv_len = 0;
 			};
 
-			rx_buf.current_write += 1;
+			rb_len = ring_buf_put(&ringbuf, buffer, recv_len);
-
+			if (rb_len < recv_len) {
-			if (rx_buf.current_write >= RX_BUF_SIZE) {
+				LOG_ERR("Drop %u bytes", recv_len - rb_len);
-				rx_buf.current_write = 0;
-				rx_buf.current_read = (RX_BUF_SIZE - 1);
-			}
-			else {
-				rx_buf.current_read = (rx_buf.current_write - 1);
 			}
 
-			// Give semaphore for usb read thread
+			k_sem_give(&rx_semaphore);
-			k_sem_give(&rx_buf.semaphore);
 		}
 	}
 }
@@ -70,64 +74,81 @@ static void usb_thread(void *p1, void *p2, void *p3) {
 	ARG_UNUSED(p1);
 	ARG_UNUSED(p2);
 	ARG_UNUSED(p3);
+	struct command_message_t msg;
+	command_message_init(&msg);
 	
 	LOG_INF("USB command processing thread started");
 	
 	while (1) {
-		// Wait forever for the rx semaphore
+		k_sem_take(&rx_semaphore, K_FOREVER);
-		k_sem_take(&rx_buf.semaphore, K_FOREVER);
 
-		int read_index = rx_buf.current_read;
+		int len;
 
-		// Check the prefix
+		// While ring buffer has data
-		if (rx_buf.buffer[read_index].prefix != COMMAND_PREFIX) {
+		do {
-			LOG_ERR("COMMAND_PREFIX does not match: %d", rx_buf.buffer[read_index].prefix);
+			uint8_t buf_prefix;
+			len = ring_buf_get(&ringbuf, &buf_prefix, 1);
 
-			// Send NACK
+			if (len && (buf_prefix == COMMAND_PREFIX)) {
-			struct command_message_t nack_buf;
+				uint8_t buf_header[4];
-			command_create_nack(&nack_buf);
+				len = ring_buf_get(&ringbuf, buf_header, 4);
-			usb_send_command(&nack_buf);
-			continue;
-		}
 
-		// Check the CRC
+				if ((len == 4) && (buf_header[1] == COMMAND_ID) && (buf_header[0] <= COMMAND_DATA_SIZE)) {
-		uint8_t calculated_crc = command_calculate_crc(&rx_buf.buffer[read_index]);
+					msg.length = buf_header[0];
-		if (calculated_crc != rx_buf.buffer[read_index].crc) {
+					msg.command = buf_header[2];
-			LOG_ERR("CRC does not match:");
+					msg.crc = buf_header[3];
-			LOG_ERR("Calculated CRC: %d", calculated_crc);
-			LOG_ERR("Received CRC: %d", rx_buf.buffer[read_index].prefix);
 
-			// Send NACK
+					if (msg.length) {
-			struct command_message_t nack_buf;
+						len = ring_buf_get(&ringbuf, msg.data, msg.length);
-			command_create_nack(&nack_buf);
+					}
-			usb_send_command(&nack_buf);
 
-			continue;
+					uint8_t calculated_crc = command_calculate_crc(&msg);
-		}
+					if (calculated_crc != msg.crc) {
+						if (RETURN_ACK) {
+							// Send NACK
+							usb_send_command(&nack_msg);
+						}
+						continue;
+					}
 
-		int ret = command_handler(&rx_buf.buffer[read_index]);
+					int ret = command_handler(&msg);
-		if (ret == 0) {
+					if (ret == 0) {
-			// Send ACK
+						if (RETURN_ACK) {
-			struct command_message_t ack_buf;
+							// Send ACK
-			command_create_ack(&ack_buf);
+							usb_send_command(&ack_msg);
-			usb_send_command(&ack_buf);
+						}
-		}
+					}
-		else {
+					else {
-			// Send NACK
+						if (RETURN_ACK) {
-			struct command_message_t nack_buf;
+							// Send NACK
-			command_create_nack(&nack_buf);
+							usb_send_command(&nack_msg);
-			usb_send_command(&nack_buf);
+						}
-		}
+					}
+
+				}
+				else {
+					// Command_id did not match, ignore
+					continue;
+				}
+			}
+			else {
+				// Prefix did not match, ignore
+				continue;
+			}
+		} while (len > 0);
+
+		uart_irq_rx_enable(uart_dev);
 	}
 	
 	LOG_INF("USB command processing thread exiting");
 }
 
 int usb_init() {
-	rx_buf.current_read = 0;
+	ring_buf_init(&ringbuf, sizeof(ring_buffer), ring_buffer);
-	rx_buf.current_write = 0;
+	k_sem_init(&rx_semaphore, 0, 1);
-	k_sem_init(&rx_buf.semaphore, 1, 1);
+
-	memset(rx_buf.buffer, 0, sizeof(rx_buf.buffer));
+	command_create_ack(&ack_msg);
+	command_create_nack(&nack_msg);
 
 	int ret;
 
@@ -174,8 +195,6 @@ int usb_init() {
 }
 
 int usb_send_command(struct command_message_t *msg) {
-	int ret = 0;
-
 	if (!device_is_ready(uart_dev)) {
 		return -ENODEV;
 	}
@@ -189,5 +208,5 @@ int usb_send_command(struct command_message_t *msg) {
 		uart_poll_out(uart_dev, msg_bytes[i]);
 	}
 
-	return ret;
+	return 0;
 }

src/zbus_channels.c

@@ -1,21 +0,0 @@
-#include "zbus_channels.h"
-#include "command_message.h"
-
-
-ZBUS_CHAN_DEFINE(
-	in_command_chan,
-	struct command_message_t,
-	NULL,
-	NULL,
-	ZBUS_OBSERVERS_EMPTY,
-	ZBUS_MSG_INIT(0)
-);
-
-ZBUS_CHAN_DEFINE(
-	out_command_chan,
-	struct command_message_t,
-	NULL,
-	NULL,
-	ZBUS_OBSERVERS_EMPTY,
-	ZBUS_MSG_INIT(0)
-);

src/zbus_channels.h

@@ -1,15 +0,0 @@
-#ifndef ZBUS_CHANNELS_H
-#define ZBUS_CHANNELS_H
-
-
-#include <zephyr/kernel.h>
-#include <zephyr/zbus/zbus.h>
-
-#include "command_message.h"
-
-
-ZBUS_CHAN_DECLARE(in_command_chan);
-ZBUS_CHAN_DECLARE(out_command_chan);
-
-
-#endif // ZBUS_CHANNELS_H