Issue #43: Bug: uart reception crashes on rx line fault

Core/Src/main.c

@@ -65,7 +65,7 @@ UART_HandleTypeDef huart3;
 osThreadId_t defaultTaskHandle;
 const osThreadAttr_t defaultTask_attributes = {
   .name = "defaultTask",
-  .stack_size = 128 * 4,
+  .stack_size = 512 * 4,
   .priority = (osPriority_t) osPriorityNormal,
 };
 /* USER CODE BEGIN PV */

KPI_Rover/Communication/UARTTransport.c

@@ -61,7 +61,7 @@ void UARTTransport_receive(uint8_t * const buf, uint8_t * const size)
 {
 	static uint8_t recvBuffer[UART_TRANSPORT_RECV_BUFFER_SIZE];
 
-	(void) osMessageQueueGet(recvQ, recvBuffer, NULL, osWaitForever);
+	for ( ; osMessageQueueGet(recvQ, recvBuffer, NULL, 50) != osOK; drvUart_restart_receiver());
 
 	*size = recvBuffer[0] - 1;
 	memcpy(buf, recvBuffer + 1, *size);

KPI_Rover/Communication/drvUart.c

@@ -128,10 +128,19 @@ bool drvUart_send(uint8_t * const buf)
 	return true;
 }
 
+void drvUart_restart_receiver(void)
+{
+	if (huart3.RxState != HAL_UART_STATE_BUSY_RX)
+	{
+		// at this point callback has swapped the buffer pointers
+		(void) HAL_UARTEx_ReceiveToIdle_DMA(&huart3, activeReceiveBuffer, DRV_UART_RECEIVE_BUFFER_SIZE);
+	}
+}
+
 void HAL_UARTEx_RxEventCallback(UART_HandleTypeDef *huart, uint16_t Size)
 {
 	// first, relaunch hardware DMA reception on the other buffer
-	HAL_UARTEx_ReceiveToIdle_DMA(&huart3, sideReceiveBuffer, DRV_UART_RECEIVE_BUFFER_SIZE);
+	(void) HAL_UARTEx_ReceiveToIdle_DMA(&huart3, sideReceiveBuffer, DRV_UART_RECEIVE_BUFFER_SIZE);
 
 	// then update local references
 	{
@@ -144,7 +153,9 @@ void HAL_UARTEx_RxEventCallback(UART_HandleTypeDef *huart, uint16_t Size)
 	}
 
 	if (on_rx_cplt == NULL)
+	{
 		return;
+	}
 
 	// then process every received packet
 	{
@@ -153,16 +164,22 @@ void HAL_UARTEx_RxEventCallback(UART_HandleTypeDef *huart, uint16_t Size)
 		for (offset = 0; offset < Size; offset += 3 + sideReceiveBuffer[1 + offset]) {
 			// verify packet is not too short (1 byte for size, 1+ byte(s) for command and 2 bytes for CRC16)
 			if (sideReceiveBuffer[1 + offset] < 4)
+			{
 				// drop ALL remaining packets on failure
 				return;
+			}
 
 			// verify packet length does not overflow received frame size
 			if (1 + offset + sideReceiveBuffer[1 + offset] > Size)
+			{
 				return;
+			}
 
 			// verify packet is not corrupted
 			if (crc16(sideReceiveBuffer + 1 + offset, sideReceiveBuffer[1 + offset]))
+			{
 				return;
+			}
 
 			// exclude size of CRC16
 			sideReceiveBuffer[1 + offset] -= 2;
@@ -173,9 +190,12 @@ void HAL_UARTEx_RxEventCallback(UART_HandleTypeDef *huart, uint16_t Size)
 	}
 }
 
-void HAL_UART_TxCpltCallback(UART_HandleTypeDef *huart) {
+void HAL_UART_TxCpltCallback(UART_HandleTypeDef *huart)
+{
 	if (on_tx_cplt == NULL)
+	{
 		return;
+	}
 
 	on_tx_cplt();
 }

KPI_Rover/Communication/drvUart.h

@@ -10,6 +10,7 @@ bool drvUart_start(void);
 bool drvUart_set_on_rx_cplt(void (*f)(const uint8_t * const buffer));
 bool drvUart_set_on_tx_cplt(void (*f)(void));
 bool drvUart_send(uint8_t *buf);
+void drvUart_restart_receiver(void);
 
 
 #endif /* __DRV_UART_H */

KPI_Rover/Database/ulDatabase.h

@@ -31,6 +31,10 @@ enum ulDatabase_ParamId {
 	MOTOR_RR_KP,
 	MOTOR_RR_KI,
 	MOTOR_RR_KD,
+	ADC_CAL_CH_11_OFFSET,
+	ADC_CAL_CH_TEMP_OFFSET,
+	ADC_CAL_CH_11_SCALE,
+	ADC_CAL_CH_TEMP_SCALE,
 	MOTOR_FL_SETPOINT,
 	MOTOR_FR_SETPOINT,
 	MOTOR_RL_SETPOINT,
@@ -58,10 +62,6 @@ enum ulDatabase_ParamId {
 	ADC_CALIBRATION_START,
 	ADC_CALIBRATION_POINT,
 	ADC_CALIBRATION_POINT_VALUE,
-	ADC_CAL_CH_11_OFFSET,
-	ADC_CAL_CH_TEMP_OFFSET,
-	ADC_CAL_CH_11_SCALE,
-	ADC_CAL_CH_TEMP_SCALE,
 	ADC_CALIBRATION_CHANNEL_ID,
 	PARAM_BATTERY_VOLTAGE,
 	PARAM_MCU_TEMPERATURE,

KPI_Rover/KPIRover.c

@@ -27,6 +27,10 @@ static struct ulDatabase_ParamMetadata ulDatabase_params[] = {
 		{0, FLOAT, true, 0.046f}, // MOTOR_RR_KP
 		{0, FLOAT, true, 0.013f}, // MOTOR_RR_KI
 		{0, FLOAT, true, 0.0001f}, // MOTOR_RR_KD
+		{0, INT32, true, 0}, // ADC_CAL_CH_11_OFFSET,
+		{0, INT32, true, 0}, // ADC_CAL_CH_TEMP_OFFSET,
+		{0, FLOAT, true, 0.0f}, // ADC_CAL_CH_11_SCALE,
+		{0, FLOAT, true, 0.0f}, // ADC_CAL_CH_TEMP_SCALE,
 		{0, INT32, false, 0}, // MOTOR_FL_SETPOINT,
 		{0, INT32, false, 0}, // MOTOR_FR_SETPOINT,
 		{0, INT32, false, 0}, // MOTOR_RL_SETPOINT,
@@ -54,10 +58,6 @@ static struct ulDatabase_ParamMetadata ulDatabase_params[] = {
 		{0, UINT8, false, 0}, // ADC_CALIBRATION_START,
 		{0, UINT8, false, 0}, // ADC_CALIBRATION_POINT,
 		{0, FLOAT, false, 0}, // ADC_CALIBRATION_POINT_VALUE,
-		{0, INT32, true, 0}, // ADC_CAL_CH_11_OFFSET,
-		{0, INT32, true, 0}, // ADC_CAL_CH_TEMP_OFFSET,
-		{0, FLOAT, true, 0.0f}, // ADC_CAL_CH_11_SCALE,
-		{0, FLOAT, true, 0.0f}, // ADC_CAL_CH_TEMP_SCALE,
 		{0, UINT8, false, 0}, // ADC_CALIBRATION_CHANNEL_ID
 		{0, FLOAT, false, 0}, // PARAM_BATTERY_VOLTAGE,
 		{0, FLOAT, false, 0}, // PARAM_MCU_TEMPERATURE

KPI_Rover/Logger/ul_ulog.c

@@ -36,7 +36,7 @@
 #include <stdio.h>
 #include <string.h>
 
-#define LOG_QUEUE_LENGTH    10
+#define LOG_QUEUE_LENGTH    20
 #define MAX_LOG_MESSAGE_SIZE 256
 
 static void ulogTask(void *argument);
@@ -51,6 +51,10 @@ osEventFlagsId_t ulogFlags;
 #define ULOG_CDC_TRANSMIT_FINISH_FLAG 0x1
 #define ULOG_DEV_CONNECTED_WAKEUP_FLAG 0x2
 
+static StaticSemaphore_t ulUlog_mutex_storage;
+#define ULOG_LOCK() osMutexAcquire(&ulUlog_mutex_storage, osWaitForever)
+#define ULOG_UNLOCK() osMutexRelease(&ulUlog_mutex_storage)
+
 static const osThreadAttr_t ulogTask_attributes = {
   .name = "ulogTask",
   .stack_size = 128 * 4,
@@ -59,8 +63,7 @@ static const osThreadAttr_t ulogTask_attributes = {
 
 
 void ul_ulog_init()
-{ 
-
+{
   xULogQueue = xQueueCreateStatic(LOG_QUEUE_LENGTH, MAX_LOG_MESSAGE_SIZE,
                                ucUlogQueueStorage, &xUlogQueueBuffer);
   if (xULogQueue == NULL)
@@ -76,16 +79,21 @@ void ul_ulog_init()
 
   ulogFlags = osEventFlagsNew(NULL);
   ulogUsbIsEstablished = 0;
+
+  {
+    const static osMutexAttr_t mutex_attrs = {NULL, 0, &ulUlog_mutex_storage, sizeof(ulUlog_mutex_storage)};
+    (void) osMutexNew(&mutex_attrs);
+  }
 }
 
-void ul_ulog_send(ulog_level_t level, const char *filename, char *msg)
+static inline void ul_ulog_send_critical(ulog_level_t level, const char *filename, char *msg)
 {
   if (xULogQueue == NULL || msg == NULL)
   {
     return;
   }
 
-  char logBuffer[MAX_LOG_MESSAGE_SIZE];
+  static char logBuffer[MAX_LOG_MESSAGE_SIZE];
   int len;
   size_t msg_len = strlen(msg);
 
@@ -136,11 +144,18 @@ void ul_ulog_send(ulog_level_t level, const char *filename, char *msg)
   }
 }
 
+void ul_ulog_send(ulog_level_t level, const char *filename, char *msg)
+{
+	ULOG_LOCK();
+	ul_ulog_send_critical(level, filename, msg);
+	ULOG_UNLOCK();
+}
+
 static void ulogTask(void *argument)
 {
   for (;;)
   {
-    char logMessage[MAX_LOG_MESSAGE_SIZE];
+    static char logMessage[MAX_LOG_MESSAGE_SIZE];
     BaseType_t result = xQueueReceive(xULogQueue, logMessage, portMAX_DELAY);
 
     if (result == pdTRUE)

KPI_Rover/Motors/PCA9685.c

@@ -16,15 +16,23 @@ extern I2C_HandleTypeDef hi2c3;
 /**
  * Helper: Write a single byte to a register.
  */
-static void pca9685_write(uint8_t reg, uint8_t data)
+#define pca9685_write(reg, data) do \
+	{ \
+        if (pca9685_write_i2c(reg, data) == false) \
+        { \
+            return false; \
+        } \
+    } while (0)
+
+static bool pca9685_write_i2c(uint8_t reg, uint8_t data)
 {
-    HAL_I2C_Mem_Write(&hi2c3,
-                      PCA9685_ADDR,
-                      reg,
-                      I2C_MEMADD_SIZE_8BIT,
-                      &data,
-                      1,
-                      HAL_MAX_DELAY);
+    return HAL_I2C_Mem_Write(&hi2c3,
+                             PCA9685_ADDR,
+                             reg,
+                             I2C_MEMADD_SIZE_8BIT,
+                             &data,
+                             1,
+                             10) == HAL_OK;
 }
 
 /**
@@ -46,14 +54,14 @@ static void pca9685_write4(uint8_t reg, uint16_t on, uint16_t off)
                       I2C_MEMADD_SIZE_8BIT,
                       buf,
                       4,
-                      HAL_MAX_DELAY);
+                      10);
 }
 
 /**
  * Initialize the PCA9685 chip.
  * Setup frequency and enable auto-increment mode.
  */
-void PCA9685_Init(void)
+bool PCA9685_Init(void)
 {
     /* 1. Reset device (Wake up) */
     pca9685_write(MODE1, 0x00);
@@ -77,6 +85,8 @@ void PCA9685_Init(void)
      */
     pca9685_write(MODE1, 0xA1);
     HAL_Delay(5);
+
+    return true;
 }
 
 /**

KPI_Rover/Motors/PCA9685.h

@@ -3,8 +3,9 @@
 
 #pragma once
 #include <stdint.h>
+#include <stdbool.h>
 
-void PCA9685_Init(void);
+bool PCA9685_Init(void);
 void PCA9685_SetPWM(uint8_t channel, uint16_t on, uint16_t off);
 void PCA9685_SetPin(uint8_t channel, uint8_t val);
 

KPI_Rover/Motors/ulMotorsController.c

@@ -5,6 +5,7 @@
 #include "Motors/PCA9685.h"
 #include <Database/ulDatabase.h>
 #include <stdio.h>
+#include <stdbool.h>
 #include <string.h>
 #include "usbd_cdc_if.h"
 
@@ -299,7 +300,10 @@ void ulMotorsController_Task(void* argument)
 
     ulMotorsController_Init(&g_motors_ctrl);
 
-    PCA9685_Init();
+    if (!PCA9685_Init()) {
+       ULOG_ERROR("Failed to initialize PCA9685");
+       osThreadExit();
+    }
 
     const osTimerAttr_t timer_attrs = {
    	   .name = "Motors_Timer"