From 770b88daacc6d87a33b545b468cc42b439840fb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Sim=C3=B5es?= Date: Wed, 11 Aug 2021 17:25:02 +0100 Subject: [PATCH 1/2] Fix RX in TI SimpleLink - Add RX ring buffer. - Update declarations and target config files. - Fix initialization of UART PAL struct. - Replace implementation of RX with blocking task because SimpleLink UART2 API doesn't have an RX event. --- .../TI_CC1352R1_LAUNCHXL/target_common.h.in | 4 +- .../target_system_io_serial_config.cpp | 5 + .../target_system_io_serial_config.h | 3 +- ...er_native_System_IO_Ports_SerialDevice.cpp | 184 +++++++++++------- .../sys_io_ser_native_target.h | 11 +- 5 files changed, 133 insertions(+), 74 deletions(-) diff --git a/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_common.h.in b/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_common.h.in index ba7f925485..2ec2eb1118 100644 --- a/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_common.h.in +++ b/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_common.h.in @@ -50,9 +50,9 @@ ///////////////////////////////////// #if defined(DEBUG) - #define MANAGED_HEAP_SIZE (28*1024) + #define MANAGED_HEAP_SIZE (23*1024) #else - #define MANAGED_HEAP_SIZE (37*1024) + #define MANAGED_HEAP_SIZE (36*1024) #endif ///////////////////////////////////// diff --git a/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.cpp b/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.cpp index f0707ec696..1f9f341187 100644 --- a/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.cpp +++ b/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.cpp @@ -15,3 +15,8 @@ __attribute__((aligned(32))) #endif uint8_t Uart1_TxBuffer[UART1_TX_SIZE]; + +#if defined(__GNUC__) +__attribute__((aligned (32))) +#endif +uint8_t Uart1_RxBuffer[UART1_RX_SIZE]; diff --git a/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.h b/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.h index 26eb12188b..81ffcc0057 100644 --- a/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.h +++ b/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.h @@ -11,4 +11,5 @@ #define NF_SERIAL_COMM_TI_USE_UART1 TRUE // buffers size -#define UART1_TX_SIZE 256 +#define UART1_TX_SIZE 256 +#define UART1_RX_SIZE 256 diff --git a/targets/TI-SimpleLink/_nanoCLR/System.IO.Ports/sys_io_ser_native_System_IO_Ports_SerialDevice.cpp b/targets/TI-SimpleLink/_nanoCLR/System.IO.Ports/sys_io_ser_native_System_IO_Ports_SerialDevice.cpp index 6f13c69609..14282ebc25 100644 --- a/targets/TI-SimpleLink/_nanoCLR/System.IO.Ports/sys_io_ser_native_System_IO_Ports_SerialDevice.cpp +++ b/targets/TI-SimpleLink/_nanoCLR/System.IO.Ports/sys_io_ser_native_System_IO_Ports_SerialDevice.cpp @@ -21,6 +21,15 @@ NF_PAL_UART Uart1_PAL; // UART number 0, 1 (..) to the port index 1, 2, etc #define UART_NUM_TO_PORT_INDEX(uartNum) ((uartNum) + 1) +// UART RX operation timeout +#define UART_TX_TIMEOUT_MILLISECONDS 5000 + +// Stack size in bytes +#define SERIAL_TASKSTACKSIZE 1024 +// better declare the task stack statically to check allocation +uint8_t SerialRxHandlerStack[SERIAL_TASKSTACKSIZE]; +Task_Struct SerialRxTaskStruct; + NF_PAL_UART *GetPalUartFromUartNum(int uart_num) { NF_PAL_UART *palUart = NULL; @@ -45,14 +54,23 @@ void UnitializePalUart(NF_PAL_UART *palUart) { if (palUart && palUart->UartDriver) { + // destroy task UART task + if (palUart->WorkingTask != NULL) + { + Task_destruct(&SerialRxTaskStruct); + } + UART2_close(palUart->UartDriver); - // free buffers meory + // free buffers memory platform_free(palUart->TxBuffer); + platform_free(palUart->RxBuffer); // null all pointers palUart->TxBuffer = NULL; + palUart->RxBuffer = NULL; palUart->UartDriver = NULL; + palUart->WorkingTask = NULL; palUart->WatchChar = 0; } } @@ -107,69 +125,72 @@ static void TxCallback(UART2_Handle handle, void *buf, size_t count, void *userA NATIVE_INTERRUPT_END } -// This callback is invoked when a character is received but the application was not ready to receive it, the character -// is passed as parameter. -static void RxCallback(UART2_Handle handle, void *buf, size_t count, void *userArg, int_fast16_t status) +void SerialRxTask(UArg a0, UArg a1) { - NATIVE_INTERRUPT_START - + (void)a1; size_t bufferIndex = 0; - uint8_t *bufferCursor = (uint8_t *)buf; + uint8_t input; + size_t bytesRead; bool watchCharFound = false; + int_fast16_t status = 1; - NF_PAL_UART *palUart = (NF_PAL_UART *)userArg; + NF_PAL_UART *palUart = GetPalUartFromUartNum((int)a0); - if (status == UART2_STATUS_SUCCESS) + while (1) { - // store this into the UART Rx buffer - // // push char to ring buffer - // // don't care about the success of the operation, if it's full we are droping the char anyway - // palUart->RxRingBuffer.Push((uint8_t *)buf, count); + status = UART2_read(palUart->UartDriver, &input, 1, &bytesRead); - if (palUart->WatchChar != 0) + if (status == UART2_STATUS_SUCCESS) { - while (bufferIndex < count) + // store this into the UART Rx buffer + + // push char to ring buffer + // don't care about the success of the operation, if it's full we are droping the char anyway + palUart->RxRingBuffer.Push((uint8_t)input); + + if (palUart->WatchChar != 0) { - if (*bufferCursor == palUart->WatchChar) + while (bufferIndex < bytesRead) { - watchCharFound = true; + if (input == palUart->WatchChar) + { + watchCharFound = true; - break; + break; + } } } - } - // is there a read operation going on? - if (palUart->RxBytesToRead > 0) - { - // yes - // check if the requested bytes are available in the buffer... - //... or if the watch char was received - // if (palUart->RxRingBuffer.Length() >= palUart->RxBytesToRead || watchCharFound) + // is there a read operation going on? + if (palUart->RxBytesToRead > 0) { - // reset Rx bytes to read count - palUart->RxBytesToRead = 0; + // yes + // check if the requested bytes are available in the buffer... + //... or if the watch char was received + if (palUart->RxRingBuffer.Length() >= palUart->RxBytesToRead || watchCharFound) + { + // reset Rx bytes to read count + palUart->RxBytesToRead = 0; - // fire event for Rx buffer complete - Events_Set(SYSTEM_EVENT_FLAG_COM_IN); + // fire event for Rx buffer complete + Events_Set(SYSTEM_EVENT_FLAG_COM_IN); + } + } + else + { + // no read operation ongoing, so fire an event + + // post a managed event with the port index and event code (check if this is the watch char or just + // another another) + PostManagedEvent( + EVENT_SERIAL, + 0, + UART_NUM_TO_PORT_INDEX(palUart->UartNum), + watchCharFound ? SerialData_WatchChar : SerialData_Chars); } - } - else - { - // no read operation ongoing, so fire an event - - // post a managed event with the port index and event code (check if this is the watch char or just another - // another) - PostManagedEvent( - EVENT_SERIAL, - 0, - UART_NUM_TO_PORT_INDEX(palUart->UartNum), - watchCharFound ? SerialData_WatchChar : SerialData_Chars); } } - - NATIVE_INTERRUPT_END } HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::get_BytesToRead___I4(CLR_RT_StackFrame &stack) @@ -193,7 +214,7 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::get_BytesToRead___ } // get length of Rx ring buffer - stack.SetResult_U4(UART2_getRxCount(palUart->UartDriver)); + stack.SetResult_U4(palUart->RxRingBuffer.Length()); NANOCLR_NOCLEANUP(); } @@ -247,18 +268,30 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeInit___VOID( } #if defined(NF_SERIAL_COMM_TI_USE_UART1) && (NF_SERIAL_COMM_TI_USE_UART1 == TRUE) - // alloc buffer memory - palUart->TxBuffer = (uint8_t *)Uart1_TxBuffer; + // assign buffers, if not already done + if (palUart->TxBuffer == NULL && palUart->RxBuffer == NULL) + { + palUart->TxBuffer = (uint8_t *)platform_malloc(UART1_TX_SIZE); + palUart->RxBuffer = (uint8_t *)platform_malloc(UART1_TX_SIZE); + + // check allocation + if (palUart->TxBuffer == NULL || palUart->RxBuffer == NULL) + { + NANOCLR_SET_AND_LEAVE(CLR_E_OUT_OF_MEMORY); + } - // init buffer - palUart->TxRingBuffer.Initialize(palUart->TxBuffer, UART1_TX_SIZE); + // init buffers + palUart->TxRingBuffer.Initialize(palUart->TxBuffer, UART1_TX_SIZE); + palUart->RxRingBuffer.Initialize(palUart->RxBuffer, UART1_RX_SIZE); + } #else #error "UART1 NOT CONFIGURED. Check TI SYSCONFIG." #endif // all the rest - palUart->UartNum = uartNum; palUart->WatchChar = 0; + palUart->RxBytesToRead = 0; + palUart->TxOngoingCount = 0; } NANOCLR_NOCLEANUP(); } @@ -283,6 +316,9 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeConfig___VOI NANOCLR_SET_AND_LEAVE(CLR_E_INVALID_PARAMETER); } + // store UART index + palUart->UartNum = uartNum; + // setup configuration // Check RS485 mode is not selected as currently not supported @@ -296,15 +332,15 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeConfig___VOI // set R/W mode with callbacks palUart->UartParams.writeMode = UART2_Mode_CALLBACK; - palUart->UartParams.readMode = UART2_Mode_CALLBACK; + palUart->UartParams.readMode = UART2_Mode_BLOCKING; // store pointer to PAL UART palUart->UartParams.userArg = palUart; - palUart->UartParams.readCallback = RxCallback; + // palUart->UartParams.readCallback = RxCallback; palUart->UartParams.writeCallback = TxCallback; - palUart->UartParams.readReturnMode = UART2_ReadReturnMode_PARTIAL; + palUart->UartParams.readReturnMode = UART2_ReadReturnMode_FULL; palUart->UartParams.baudRate = (int)pThis[FIELD___baudRate].NumericByRef().s4; // UART2_DataLen goes from 5->0 to 8->3 bits. @@ -337,18 +373,43 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeConfig___VOI { // stop UART before changing configuration, just in case UART2_close(palUart->UartDriver); + + // destroy task UART task, if it's running + if (palUart->WorkingTask != NULL) + { + Task_destruct(&SerialRxTaskStruct); + + // null pointer + palUart->WorkingTask = NULL; + } } palUart->UartDriver = UART2_open(uartNum, &palUart->UartParams); - UART2_rxEnable(palUart->UartDriver); - // check if UART was opened if (palUart->UartDriver == NULL) { NANOCLR_SET_AND_LEAVE(CLR_E_INVALID_OPERATION); } + // OK to enable RX on UART now + UART2_rxEnable(palUart->UartDriver); + + // create RX task + // this can be replaced with an RX handler when TI decides to add a decent RX event to their API + Task_Params taskParams; + Task_Params_init(&taskParams); + taskParams.stackSize = SERIAL_TASKSTACKSIZE; + taskParams.priority = 4; + taskParams.stack = &SerialRxHandlerStack; + taskParams.arg0 = (UArg)palUart->UartNum; + + // construct task + Task_construct(&SerialRxTaskStruct, (Task_FuncPtr)SerialRxTask, &taskParams, NULL); + + // store pointer to task + palUart->WorkingTask = Task_handle(&SerialRxTaskStruct); + NANOCLR_NOCLEANUP(); } @@ -628,7 +689,7 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeRead___U4__S NANOCLR_CHECK_HRESULT(stack.SetupTimeoutFromTicks(hbTimeout, timeoutTicks)); // figure out what's available in the Rx ring buffer - if (UART2_getRxCount(palUart->UartDriver) >= count) + if (palUart->RxRingBuffer.Length() >= count) { // read from Rx ring buffer bytesToRead = count; @@ -680,14 +741,7 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeRead___U4__S if (!eventResult) { // event timeout - - bytesToRead = count; - - if (count > UART2_getRxCount(palUart->UartDriver)) - { - // need to adjust because there aren't enough bytes available - bytesToRead = UART2_getRxCount(palUart->UartDriver); - } + NANOCLR_SET_AND_LEAVE(CLR_E_TIMEOUT); } } } @@ -695,7 +749,7 @@ HRESULT Library_sys_io_ser_native_System_IO_Ports_SerialPort::NativeRead___U4__S if (bytesToRead > 0) { // pop the requested bytes from the ring buffer - UART2_read(palUart->UartDriver, data, bytesToRead, &bytesRead); + bytesRead = palUart->RxRingBuffer.Pop(data, bytesToRead); } // pop timeout heap block from stack diff --git a/targets/TI-SimpleLink/_nanoCLR/System.IO.Ports/sys_io_ser_native_target.h b/targets/TI-SimpleLink/_nanoCLR/System.IO.Ports/sys_io_ser_native_target.h index cc2baac8b0..ee32ebf2ab 100644 --- a/targets/TI-SimpleLink/_nanoCLR/System.IO.Ports/sys_io_ser_native_target.h +++ b/targets/TI-SimpleLink/_nanoCLR/System.IO.Ports/sys_io_ser_native_target.h @@ -19,10 +19,14 @@ typedef struct UART2_Params UartParams; uint8_t UartNum; + Task_Handle WorkingTask; + HAL_RingBuffer TxRingBuffer; uint8_t *TxBuffer; uint16_t TxOngoingCount; + HAL_RingBuffer RxRingBuffer; + uint8_t *RxBuffer; uint16_t RxBytesToRead; bool IsLongRunning; @@ -37,12 +41,7 @@ typedef struct extern NF_PAL_UART Uart1_PAL; #endif -///////////////////////////////////// -// UART Tx buffers // -// these live in the target folder // -///////////////////////////////////// -extern uint8_t Uart1_TxBuffer[]; - #define UART_TX_BUFFER_SIZE(num) UART##num##_TX_SIZE +#define UART_RX_BUFFER_SIZE(num) UART##num##_RX_SIZE #endif //_SYS_IO_SER_NATIVE_TARGET_H_ From b6cdf4c867ecc2ad00d86358655efdf1b6664b18 Mon Sep 17 00:00:00 2001 From: nfbot Date: Wed, 11 Aug 2021 17:50:46 +0100 Subject: [PATCH 2/2] Code style fixes (#93) Automated fixes for code style. --- .../TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.cpp | 2 +- .../TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.cpp b/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.cpp index 1f9f341187..6ff63c217e 100644 --- a/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.cpp +++ b/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.cpp @@ -17,6 +17,6 @@ __attribute__((aligned(32))) uint8_t Uart1_TxBuffer[UART1_TX_SIZE]; #if defined(__GNUC__) -__attribute__((aligned (32))) +__attribute__((aligned(32))) #endif uint8_t Uart1_RxBuffer[UART1_RX_SIZE]; diff --git a/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.h b/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.h index 81ffcc0057..48d99451b2 100644 --- a/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.h +++ b/targets/TI-SimpleLink/TI_CC1352R1_LAUNCHXL/target_system_io_serial_config.h @@ -11,5 +11,5 @@ #define NF_SERIAL_COMM_TI_USE_UART1 TRUE // buffers size -#define UART1_TX_SIZE 256 -#define UART1_RX_SIZE 256 +#define UART1_TX_SIZE 256 +#define UART1_RX_SIZE 256