From 3654bf2206758ca214e3128013b645fb13d82c54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Hru=C5=A1ka?= Date: Mon, 5 Mar 2018 11:38:25 +0100 Subject: [PATCH] various speed-ups in TF_WriteImpl and elsewhere --- TinyFrame/TF_Integration.c | 62 +++++++++++++++++--------------------- TinyFrame/TinyFrame.c | 10 ++++-- TinyFrame/TinyFrame.h | 2 +- USB/usbd_cdc_if.c | 2 +- comm/messages.c | 8 ++--- freertos.c | 2 +- units/adc/_adc_core.c | 3 +- 7 files changed, 43 insertions(+), 46 deletions(-) diff --git a/TinyFrame/TF_Integration.c b/TinyFrame/TF_Integration.c index 22e0b9f..fb911df 100644 --- a/TinyFrame/TF_Integration.c +++ b/TinyFrame/TF_Integration.c @@ -4,48 +4,40 @@ // TinyFrame integration // -#include #include "platform.h" #include "task_main.h" #include "USB/usbd_cdc_if.h" +#include "USB/usb_device.h" #include "TinyFrame.h" extern osSemaphoreId semVcomTxReadyHandle; extern osMutexId mutTinyFrameTxHandle; -static volatile bool first_tx = false; // XXX global - void TF_WriteImpl(TinyFrame *tf, const uint8_t *buff, uint32_t len) { #if 1 -// if (!first_tx) { -// // wait for the last USB transmission to be finished -// int32_t mxStatus; -// mxStatus = osSemaphoreWait(semVcomTxReadyHandle, 100); -// if (mxStatus != osOK) { -// TF_Error("Tx stalled"); -// return; -// } -// } -// first_tx = false; - - // Padding to a multiple of 64 bytes + const uint32_t real_size = len; + + // Padding to a multiple of 64 bytes - this is supposed to maximize the bulk transfer speed if (len&0x3F) { uint32_t pad = (64 - (len&0x3F)); memset((void *) (buff + len), 0, pad); len += pad; // padding to a multiple of 64 (size of the endpoint) } + // We bypass the USBD driver library's overhead by using the HAL function directly assert_param(HAL_OK == HAL_PCD_EP_Transmit(hUsbDeviceFS.pData, CDC_IN_EP, (uint8_t *) buff, len)); - // Wait for the semaphore - HAL keeps a pointer to the buffer, and it's the TinyFrame Tx buffer, - // so if we let it process it in the background, it could get corrupted before the Tx is completed. - int32_t mxStatus; - mxStatus = osSemaphoreWait(semVcomTxReadyHandle, 100); - if (mxStatus != osOK) { - TF_Error("Tx stalled"); - return; + // The buffer is the TF transmit buffer, we can't leave it to work asynchronously because + // the next call could modify it before it's been transmitted (in the case of a chunked / multi-part frame) + + // the assumption here is that all until the last chunk use the full buffer capacity + if (real_size == TF_SENDBUF_LEN) { + if (pdTRUE != xSemaphoreTake(semVcomTxReadyHandle, 100)) { + TF_Error("Tx stalled in WriteImpl"); + return; + } } #else (void) tf; @@ -76,20 +68,18 @@ void TF_WriteImpl(TinyFrame *tf, const uint8_t *buff, uint32_t len) bool TF_ClaimTx(TinyFrame *tf) { (void) tf; -// assert_param(osThreadGetId() != tskMainHandle); - assert_param(!inIRQ()); - - assert_param(osOK == osMutexWait(mutTinyFrameTxHandle, 5000)); +// assert_param(!inIRQ()); // useless delay + assert_param(pdTRUE == xSemaphoreTake(mutTinyFrameTxHandle, 5000)); // trips the wd -// // wait for the last USB transmission to be finished -// int32_t mxStatus; -// mxStatus = osSemaphoreWait(semVcomTxReadyHandle, 100); -// if (mxStatus != osOK) { -// TF_Error("Tx stalled"); -// return false; -// } + // The last chunk from some previous frame may still be being transmitted, + // wait for it to finish (the semaphore is given in the CDC tx done handler) + if (pdTRUE != xSemaphoreTake(semVcomTxReadyHandle, 100)) { + TF_Error("Tx stalled in Claim"); - first_tx = true; + // release the guarding mutex again + assert_param(pdTRUE == xSemaphoreGive(mutTinyFrameTxHandle)); + return false; + } return true; } @@ -98,5 +88,7 @@ bool TF_ClaimTx(TinyFrame *tf) void TF_ReleaseTx(TinyFrame *tf) { (void) tf; - assert_param(osOK == osMutexRelease(mutTinyFrameTxHandle)); + assert_param(pdTRUE == xSemaphoreGive(mutTinyFrameTxHandle)); + + // the last payload is sent asynchronously } diff --git a/TinyFrame/TinyFrame.c b/TinyFrame/TinyFrame.c index 3489942..11f14b0 100644 --- a/TinyFrame/TinyFrame.c +++ b/TinyFrame/TinyFrame.c @@ -870,7 +870,11 @@ static inline uint32_t _TF_FN TF_ComposeTail(uint8_t *outbuff, TF_CKSUM *cksum) */ static bool _TF_FN TF_SendFrame_Begin(TinyFrame *tf, TF_Msg *msg, TF_Listener listener, TF_TICKS timeout) { - TF_TRY(TF_ClaimTx(tf)); + bool suc = TF_ClaimTx(tf); + if (!suc) { + TF_Error("TF lock not free"); + return false; + } tf->tx_pos = (uint32_t) TF_ComposeHead(tf, tf->sendbuf, msg); // frame ID is incremented here if it's not a response tf->tx_len = msg->len; @@ -1031,10 +1035,10 @@ bool _TF_FN TF_Query_Multipart(TinyFrame *tf, TF_Msg *msg, TF_Listener listener, return TF_Query(tf, msg, listener, timeout); } -void _TF_FN TF_Respond_Multipart(TinyFrame *tf, TF_Msg *msg) +bool _TF_FN TF_Respond_Multipart(TinyFrame *tf, TF_Msg *msg) { msg->data = NULL; - TF_Respond(tf, msg); + return TF_Respond(tf, msg); } void _TF_FN TF_Multipart_Payload(TinyFrame *tf, const uint8_t *buff, uint32_t length) diff --git a/TinyFrame/TinyFrame.h b/TinyFrame/TinyFrame.h index 5f6943c..7766aa1 100644 --- a/TinyFrame/TinyFrame.h +++ b/TinyFrame/TinyFrame.h @@ -369,7 +369,7 @@ bool TF_Query_Multipart(TinyFrame *tf, TF_Msg *msg, TF_Listener listener, TF_TIC * TF_Respond() with multipart payload. * msg.data is ignored and set to NULL */ -void TF_Respond_Multipart(TinyFrame *tf, TF_Msg *msg); +bool TF_Respond_Multipart(TinyFrame *tf, TF_Msg *msg); /** * Send the payload for a started multipart frame. This can be called multiple times diff --git a/USB/usbd_cdc_if.c b/USB/usbd_cdc_if.c index b6ee7eb..b636985 100644 --- a/USB/usbd_cdc_if.c +++ b/USB/usbd_cdc_if.c @@ -314,7 +314,7 @@ void USBD_CDC_TransmitDone(USBD_HandleTypeDef *pdev) assert_param(inIRQ()); portBASE_TYPE taskWoken = pdFALSE; - assert_param(xSemaphoreGiveFromISR(semVcomTxReadyHandle, &taskWoken) == pdTRUE); + assert_param(pdTRUE == xSemaphoreGiveFromISR(semVcomTxReadyHandle, &taskWoken)); portYIELD_FROM_ISR(taskWoken); } /* USER CODE END PRIVATE_FUNCTIONS_IMPLEMENTATION */ diff --git a/comm/messages.c b/comm/messages.c index 620a17e..7675504 100644 --- a/comm/messages.c +++ b/comm/messages.c @@ -65,7 +65,7 @@ static void settings_bulkread_cb(BulkRead *bulk, uint32_t chunk, uint8_t *buffer if (buffer == NULL) { free_ck(bulk); iw_end(); - dbg("INI read complete."); +// dbg("INI read complete."); return; } @@ -81,7 +81,7 @@ static void settings_bulkread_cb(BulkRead *bulk, uint32_t chunk, uint8_t *buffer */ static TF_Result lst_ini_export(TinyFrame *tf, TF_Msg *msg) { - dbg("Bulk read INI file"); +// dbg("Bulk read INI file"); BulkRead *bulk = malloc_ck(sizeof(BulkRead)); assert_param(bulk != NULL); @@ -114,7 +114,7 @@ static void settings_bulkwrite_cb(BulkWrite *bulk, const uint8_t *chunk, uint32_ if (bulk->offset > 0) { settings_load_ini_end(); - dbg("INI write complete"); +// dbg("INI write complete"); } else { dbg("INI write failed"); } @@ -131,7 +131,7 @@ static void settings_bulkwrite_cb(BulkWrite *bulk, const uint8_t *chunk, uint32_ */ static TF_Result lst_ini_import(TinyFrame *tf, TF_Msg *msg) { - dbg("Bulk write INI file"); +// dbg("Bulk write INI file"); BulkWrite *bulk = malloc_ck(sizeof(BulkWrite)); assert_param(bulk); diff --git a/freertos.c b/freertos.c index 10cebcf..6213905 100644 --- a/freertos.c +++ b/freertos.c @@ -169,7 +169,7 @@ void MX_FREERTOS_Init(void) { /* USER CODE BEGIN RTOS_SEMAPHORES */ /* add semaphores, ... */ -// xSemaphoreGive(semVcomTxReadyHandle); + xSemaphoreGive(semVcomTxReadyHandle); /* USER CODE END RTOS_SEMAPHORES */ /* USER CODE BEGIN RTOS_TIMERS */ diff --git a/units/adc/_adc_core.c b/units/adc/_adc_core.c index 023de49..c5e7a94 100644 --- a/units/adc/_adc_core.c +++ b/units/adc/_adc_core.c @@ -40,7 +40,8 @@ static void UADC_JobSendBlockChunk(Job *job) .len = (TF_LEN) (1 /*seq*/ + count * sizeof(uint16_t)), .type = type, }; - TF_Respond_Multipart(comm, &msg); + + assert_param(true == TF_Respond_Multipart(comm, &msg)); TF_Multipart_Payload(comm, &priv->stream_serial, 1); TF_Multipart_Payload(comm, (uint8_t *) (priv->dma_buffer + start), count * sizeof(uint16_t)); TF_Multipart_Close(comm);