From 48a843899ed527887c093e49614191411e4001f8 Mon Sep 17 00:00:00 2001
From: danukeru <danukeru@foulab.org>
Date: Sat, 24 Aug 2019 11:53:05 -0400
Subject: [PATCH] feat(serial): Funnel writes through a stream-buffer
Previously, race-conditions between multiple serial writes could lead to
memory corruption and weird artifacts like I2C getting stuck. This
patch introduces a write-streambuffer which queues all messages.
Co-authored-by: Rahix <rahix@rahix.de>
Signed-off-by: danukeru <danukeru@foulab.org>
---
epicardium/main.c | 9 +-
epicardium/modules/modules.h | 8 ++
epicardium/modules/serial.c | 170 +++++++++++++++++++++++++++++------
epicardium/usb/cdcacm.c | 8 +-
4 files changed, 162 insertions(+), 33 deletions(-)
diff --git a/epicardium/main.c b/epicardium/main.c
index be3e3519..dadad9bf 100644
--- a/epicardium/main.c
+++ b/epicardium/main.c
@@ -24,7 +24,7 @@ int main(void)
* Version Splash
*/
const char *version_buf = CARD10_VERSION;
- const int offset = (160 - (int)strlen(version_buf) * 14) / 2;
+ const int offset = (160 - (int)strlen(version_buf) * 14) / 2;
epic_disp_clear(0x3b7);
epic_disp_print(10, 20, "Epicardium", 0x290, 0x3b7);
epic_disp_print(offset > 0 ? offset : 0, 40, version_buf, 0x290, 0x3b7);
@@ -39,7 +39,7 @@ int main(void)
(const char *)"Serial",
configMINIMAL_STACK_SIZE * 2,
NULL,
- tskIDLE_PRIORITY + 1,
+ tskIDLE_PRIORITY + 3,
NULL) != pdPASS) {
LOG_CRIT("startup", "Failed to create %s task!", "Serial");
abort();
@@ -134,6 +134,11 @@ int main(void)
abort();
}
+ /*
+ * Initialize serial driver data structures.
+ */
+ serial_init();
+
LOG_DEBUG("startup", "Starting FreeRTOS ...");
vTaskStartScheduler();
diff --git a/epicardium/modules/modules.h b/epicardium/modules/modules.h
index 98b0d5e9..2d51f786 100644
--- a/epicardium/modules/modules.h
+++ b/epicardium/modules/modules.h
@@ -24,10 +24,18 @@ void return_to_menu(void);
/* ---------- Serial ------------------------------------------------------- */
#define SERIAL_READ_BUFFER_SIZE 128
+#define SERIAL_WRITE_STREAM_BUFFER_SIZE 512
+void serial_init();
void vSerialTask(void *pvParameters);
void serial_enqueue_char(char chr);
extern TaskHandle_t serial_task_id;
+// For the eSetBit xTaskNotify task semaphore trigger
+enum serial_notify{
+ SERIAL_WRITE_NOTIFY = 0x01,
+ SERIAL_READ_NOTIFY = 0x02,
+};
+
/* ---------- LED Animation / Personal States ------------------------------ */
#define PERSONAL_STATE_LED 14
void vLedTask(void *pvParameters);
diff --git a/epicardium/modules/serial.c b/epicardium/modules/serial.c
index c5350b11..3292ab93 100644
--- a/epicardium/modules/serial.c
+++ b/epicardium/modules/serial.c
@@ -10,40 +10,115 @@
#include "FreeRTOS.h"
#include "task.h"
#include "queue.h"
+#include "stream_buffer.h"
#include <stdint.h>
#include <stdio.h>
+/* The serial console in use (UART0) */
+extern mxc_uart_regs_t *ConsoleUart;
+
/* Task ID for the serial handler */
TaskHandle_t serial_task_id = NULL;
-/* The serial console in use (UART0) */
-extern mxc_uart_regs_t *ConsoleUart;
/* Read queue, filled by both UART and CDCACM */
static QueueHandle_t read_queue;
+/* Stream Buffer for handling all writes to serial */
+static StreamBufferHandle_t write_stream_buffer = NULL;
+
+void serial_init()
+{
+ /* Setup read queue */
+ static uint8_t buffer[sizeof(char) * SERIAL_READ_BUFFER_SIZE];
+ static StaticQueue_t read_queue_data;
+ read_queue = xQueueCreateStatic(
+ SERIAL_READ_BUFFER_SIZE, sizeof(char), buffer, &read_queue_data
+ );
+
+ /* Setup write queue */
+ static uint8_t ucWrite_stream_buffer[SERIAL_WRITE_STREAM_BUFFER_SIZE];
+ static StaticStreamBuffer_t xStream_buffer_struct;
+ write_stream_buffer = xStreamBufferCreateStatic(
+ sizeof(ucWrite_stream_buffer),
+ 1,
+ ucWrite_stream_buffer,
+ &xStream_buffer_struct
+ );
+}
/*
* API-call to write a string. Output goes to both CDCACM and UART
*/
void epic_uart_write_str(const char *str, intptr_t length)
{
- uint32_t basepri = __get_BASEPRI();
- if (xPortIsInsideInterrupt()) {
- taskENTER_CRITICAL_FROM_ISR();
- } else {
- taskENTER_CRITICAL();
+ if (length == 0) {
+ return;
}
- UART_Write(ConsoleUart, (uint8_t *)str, length);
+ /*
+ * Check if the stream buffer is even initialized yet
+ */
+ if (write_stream_buffer == NULL) {
+ UART_Write(ConsoleUart, (uint8_t *)str, length);
+ cdcacm_write((uint8_t *)str, length);
+ return;
+ }
if (xPortIsInsideInterrupt()) {
+ BaseType_t resched1 = pdFALSE;
+ BaseType_t resched2 = pdFALSE;
+
+ /*
+ * Enter a critial section so no other task can write to the
+ * stream buffer.
+ */
+ uint32_t basepri = __get_BASEPRI();
+ taskENTER_CRITICAL_FROM_ISR();
+
+ xStreamBufferSendFromISR(
+ write_stream_buffer, str, length, &resched1
+ );
+
taskEXIT_CRITICAL_FROM_ISR(basepri);
+
+ if (serial_task_id != NULL) {
+ xTaskNotifyFromISR(
+ serial_task_id,
+ SERIAL_WRITE_NOTIFY,
+ eSetBits,
+ &resched2
+ );
+ }
+
+ /* Yield if this write woke up a higher priority task */
+ portYIELD_FROM_ISR(resched1 || resched2);
} else {
- taskEXIT_CRITICAL();
+ size_t bytes_sent = 0;
+ size_t index = 0;
+ do {
+ taskENTER_CRITICAL();
+ /*
+ * Wait time needs to be zero, because we are in a
+ * critical section.
+ */
+ bytes_sent = xStreamBufferSend(
+ write_stream_buffer,
+ &str[index],
+ length - index,
+ 0
+ );
+ index += bytes_sent;
+ taskEXIT_CRITICAL();
+ if (serial_task_id != NULL) {
+ xTaskNotify(
+ serial_task_id,
+ SERIAL_WRITE_NOTIFY,
+ eSetBits
+ );
+ portYIELD();
+ }
+ } while (index < length);
}
-
- cdcacm_write((uint8_t *)str, length);
- ble_uart_write((uint8_t *)str, length);
}
/*
@@ -101,7 +176,12 @@ void UART0_IRQHandler(void)
static void uart_callback(uart_req_t *req, int error)
{
BaseType_t xHigherPriorityTaskWoken = pdFALSE;
- vTaskNotifyGiveFromISR(serial_task_id, &xHigherPriorityTaskWoken);
+ xTaskNotifyFromISR(
+ serial_task_id,
+ SERIAL_READ_NOTIFY,
+ eSetBits,
+ &xHigherPriorityTaskWoken
+ );
portYIELD_FROM_ISR(xHigherPriorityTaskWoken);
}
@@ -122,16 +202,8 @@ void serial_enqueue_char(char chr)
void vSerialTask(void *pvParameters)
{
- static uint8_t buffer[sizeof(char) * SERIAL_READ_BUFFER_SIZE];
- static StaticQueue_t read_queue_data;
-
serial_task_id = xTaskGetCurrentTaskHandle();
- /* Setup read queue */
- read_queue = xQueueCreateStatic(
- SERIAL_READ_BUFFER_SIZE, sizeof(char), buffer, &read_queue_data
- );
-
/* Setup UART interrupt */
NVIC_ClearPendingIRQ(UART0_IRQn);
NVIC_DisableIRQ(UART0_IRQn);
@@ -145,6 +217,9 @@ void vSerialTask(void *pvParameters)
.callback = uart_callback,
};
+ uint8_t rx_data[20];
+ size_t received_bytes;
+
while (1) {
int ret = UART_ReadAsync(ConsoleUart, &read_req);
if (ret != E_NO_ERROR && ret != E_BUSY) {
@@ -152,18 +227,55 @@ void vSerialTask(void *pvParameters)
vTaskDelay(portMAX_DELAY);
}
- ulTaskNotifyTake(pdTRUE, portTICK_PERIOD_MS * 1000);
+ ret = ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
- if (read_req.num > 0) {
- serial_enqueue_char(*read_req.data);
- }
+ if (ret & SERIAL_WRITE_NOTIFY) {
+ do {
+ received_bytes = xStreamBufferReceive(
+ write_stream_buffer,
+ (void *)rx_data,
+ sizeof(rx_data),
+ 0
+ );
+
+ if (received_bytes == 0) {
+ break;
+ }
+
+ /*
+ * The SDK-driver for UART is not reentrant
+ * which means we need to perform UART writes
+ * in a critical section.
+ */
+ taskENTER_CRITICAL();
+ UART_Write(
+ ConsoleUart,
+ (uint8_t *)&rx_data,
+ received_bytes
+ );
+ taskEXIT_CRITICAL();
- while (UART_NumReadAvail(ConsoleUart) > 0) {
- serial_enqueue_char(UART_ReadByte(ConsoleUart));
+ cdcacm_write(
+ (uint8_t *)&rx_data, received_bytes
+ );
+ ble_uart_write(
+ (uint8_t *)&rx_data, received_bytes
+ );
+ } while (received_bytes > 0);
}
- while (cdcacm_num_read_avail() > 0) {
- serial_enqueue_char(cdcacm_read());
+ if (ret & SERIAL_READ_NOTIFY) {
+ if (read_req.num > 0) {
+ serial_enqueue_char(*read_req.data);
+ }
+
+ while (UART_NumReadAvail(ConsoleUart) > 0) {
+ serial_enqueue_char(UART_ReadByte(ConsoleUart));
+ }
+
+ while (cdcacm_num_read_avail() > 0) {
+ serial_enqueue_char(cdcacm_read());
+ }
}
}
}
diff --git a/epicardium/usb/cdcacm.c b/epicardium/usb/cdcacm.c
index 679a3a05..ea046721 100644
--- a/epicardium/usb/cdcacm.c
+++ b/epicardium/usb/cdcacm.c
@@ -17,6 +17,7 @@
#include "cdc_acm.h"
#include "modules/log.h"
+#include "modules/modules.h"
#include "FreeRTOS.h"
#include "task.h"
@@ -118,8 +119,11 @@ void USB_IRQHandler(void)
if (serial_task_id != NULL) {
BaseType_t xHigherPriorityTaskWoken = pdFALSE;
- vTaskNotifyGiveFromISR(
- serial_task_id, &xHigherPriorityTaskWoken
+ xTaskNotifyFromISR(
+ serial_task_id,
+ SERIAL_READ_NOTIFY,
+ eSetBits,
+ &xHigherPriorityTaskWoken
);
portYIELD_FROM_ISR(xHigherPriorityTaskWoken);
}
--
GitLab