From 931f6cea5999e22352236121d49e48d97930a91f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Hru=C5=A1ka?= Date: Sat, 6 Jan 2018 15:53:56 +0100 Subject: [PATCH] added missing mutex for shared scratch buffer --- comm/msg_bulkread.c | 6 ++-- framework/settings.c | 8 ++++- framework/unit.c | 2 +- framework/unit.h | 2 +- framework/unit_registry.c | 63 +++++++++++++++++++++---------------- freertos.c | 6 ++++ platform/plat_compat.h | 5 +-- tasks/task_msg.c | 3 +- units/digital_in/unit_din.c | 2 +- units/i2c/unit_i2c.c | 2 +- 10 files changed, 61 insertions(+), 38 deletions(-) diff --git a/comm/msg_bulkread.c b/comm/msg_bulkread.c index 1151b23..0bc31f8 100644 --- a/comm/msg_bulkread.c +++ b/comm/msg_bulkread.c @@ -11,7 +11,7 @@ #include "utils/payload_builder.h" /** Buffer for preparing bulk chunks */ -static uint8_t bulkread_buffer[BULKREAD_MAX_CHUNK]; +static uint8_t bulkread_buffer[BULK_READ_BUF_LEN]; /** * TF listener for the bulk read transaction @@ -36,7 +36,7 @@ static TF_Result bulkread_lst(TinyFrame *tf, TF_Msg *msg) uint32_t chunk = pp_u32(&pp); chunk = MIN(chunk, bulk->len - bulk->offset); - chunk = MIN(chunk, BULKREAD_MAX_CHUNK); + chunk = MIN(chunk, BULK_READ_BUF_LEN); // load data into the buffer bulk->read(bulk, chunk, bulkread_buffer); @@ -81,7 +81,7 @@ void bulkread_start(TinyFrame *tf, BulkRead *bulk) uint8_t buf[8]; PayloadBuilder pb = pb_start(buf, 4, NULL); pb_u32(&pb, bulk->len); - pb_u32(&pb, BULKREAD_MAX_CHUNK); + pb_u32(&pb, BULK_READ_BUF_LEN); // We use userdata1 to hold a reference to the bulk transfer TF_Msg msg = { diff --git a/framework/settings.c b/framework/settings.c index 42277bd..073366b 100644 --- a/framework/settings.c +++ b/framework/settings.c @@ -223,6 +223,8 @@ static void ini_preamble(IniWriter *iw, const char *filename) iw_comment(iw, "Close the LOCK jumper to save them to Flash."); } +extern osMutexId mutScratchBufferHandle; + /** * Write system settings to INI (without section) */ @@ -230,7 +232,11 @@ void settings_build_units_ini(IniWriter *iw) { ini_preamble(iw, "UNITS.INI"); - ureg_build_ini(iw); + assert_param(osOK == osMutexWait(mutScratchBufferHandle, 5000)); + { + ureg_build_ini(iw); + } + assert_param(osOK == osMutexRelease(mutScratchBufferHandle)); } /** diff --git a/framework/unit.c b/framework/unit.c index 872b596..a1839f4 100644 --- a/framework/unit.c +++ b/framework/unit.c @@ -6,7 +6,7 @@ #include "unit.h" #include "resources.h" -char unit_tmp512[512]; +char unit_tmp512[UNIT_TMP_LEN]; // Abort partly inited unit void clean_failed_unit(Unit *unit) diff --git a/framework/unit.h b/framework/unit.h index 0cb86fa..9f85696 100644 --- a/framework/unit.h +++ b/framework/unit.h @@ -16,7 +16,7 @@ return E_BAD_UNIT_TYPE; \ } while (0) -extern char unit_tmp512[512]; // temporary static buffer - not expected to be accessed asynchronously +extern char unit_tmp512[UNIT_TMP_LEN]; // temporary static buffer - not expected to be accessed asynchronously // TODO add mutex? typedef struct unit Unit; diff --git a/framework/unit_registry.c b/framework/unit_registry.c index 7af2d51..575bbaa 100644 --- a/framework/unit_registry.c +++ b/framework/unit_registry.c @@ -431,42 +431,51 @@ uint32_t ureg_get_num_units(void) return (uint32_t) unit_count; } +extern osMutexId mutScratchBufferHandle; + /** Deliver message to it's destination unit */ void ureg_deliver_unit_request(TF_Msg *msg) { - PayloadParser pp = pp_start(msg->data, msg->len, NULL); - uint8_t callsign = pp_u8(&pp); - uint8_t command = pp_u8(&pp); + // we must claim the scratch buffer because it's used by many units internally + assert_param(osOK == osMutexWait(mutScratchBufferHandle, 5000)); + { + PayloadParser pp = pp_start(msg->data, msg->len, NULL); + uint8_t callsign = pp_u8(&pp); + uint8_t command = pp_u8(&pp); - // highest bit indicates user wants an extra confirmation on success - bool confirmed = (bool) (command & 0x80); - command &= 0x7F; + // highest bit indicates user wants an extra confirmation on success + bool confirmed = (bool) (command & 0x80); + command &= 0x7F; - if (callsign == 0 || !pp.ok) { - com_respond_error(msg->frame_id, E_MALFORMED_COMMAND); - return; - } + if (callsign == 0 || !pp.ok) { + com_respond_error(msg->frame_id, E_MALFORMED_COMMAND); + goto quit; + } - UlistEntry *li = ulist_head; - while (li != NULL) { - Unit *const pUnit = &li->unit; - if (pUnit->callsign == callsign && pUnit->status == E_SUCCESS) { - error_t rv = pUnit->driver->handleRequest(pUnit, msg->frame_id, command, &pp); - - // send extra SUCCESS confirmation message. - // error is expected to have already been reported. - if (rv == E_SUCCESS) { - if (confirmed) com_respond_ok(msg->frame_id); - } else { - com_respond_error(msg->frame_id, rv); + UlistEntry *li = ulist_head; + while (li != NULL) { + Unit *const pUnit = &li->unit; + if (pUnit->callsign == callsign && pUnit->status == E_SUCCESS) { + error_t rv = pUnit->driver->handleRequest(pUnit, msg->frame_id, command, &pp); + + // send extra SUCCESS confirmation message. + // error is expected to have already been reported. + if (rv == E_SUCCESS) { + if (confirmed) com_respond_ok(msg->frame_id); + } + else { + com_respond_error(msg->frame_id, rv); + } + goto quit; } - return; + li = li->next; } - li = li->next; - } - // Not found - com_respond_error(msg->frame_id, E_NO_SUCH_UNIT); + // Not found + com_respond_error(msg->frame_id, E_NO_SUCH_UNIT); + } + quit: + assert_param(osOK == osMutexRelease(mutScratchBufferHandle)); } /** Send a response for a unit-list request */ diff --git a/freertos.c b/freertos.c index 4d1e643..1de37d0 100644 --- a/freertos.c +++ b/freertos.c @@ -84,6 +84,9 @@ osStaticMutexDef_t mutTinyFrameTxControlBlock; osSemaphoreId semVcomTxReadyHandle; osStaticSemaphoreDef_t semVcomTxReadyControlBlock; +osMutexId mutScratchBufferHandle; +osStaticMutexDef_t mutScratchBufferControlBlock; + /* USER CODE BEGIN Variables */ /* USER CODE END Variables */ @@ -142,6 +145,9 @@ void MX_FREERTOS_Init(void) { osMutexStaticDef(mutTinyFrameTx, &mutTinyFrameTxControlBlock); mutTinyFrameTxHandle = osMutexCreate(osMutex(mutTinyFrameTx)); + osMutexStaticDef(mutScratchBuffer, &mutScratchBufferControlBlock); + mutScratchBufferHandle = osMutexCreate(osMutex(mutScratchBuffer)); + /* USER CODE BEGIN RTOS_MUTEX */ /* add mutexes, ... */ /* USER CODE END RTOS_MUTEX */ diff --git a/platform/plat_compat.h b/platform/plat_compat.h index 0b46caa..c8e0627 100644 --- a/platform/plat_compat.h +++ b/platform/plat_compat.h @@ -9,10 +9,11 @@ // -------- Static buffers --------- #define TSK_STACK_MAIN 220 // USB / VFS task stack size -#define TSK_STACK_MSG 200 // TF message handler task stack size +#define TSK_STACK_MSG 220 // TF message handler task stack size #define TSK_STACK_JOBRUNNER 80 // Job runner task stack size -#define BULKREAD_MAX_CHUNK 256 // Bulk read buffer +#define BULK_READ_BUF_LEN 256 // Buffer for TF bulk reads +#define UNIT_TMP_LEN 512 // Buffer for bulk read and varions internal unit operations #define FLASH_SAVE_BUF_LEN 256 // Static buffer for saving to flash diff --git a/tasks/task_msg.c b/tasks/task_msg.c index 88d1fb4..f1fcc2d 100644 --- a/tasks/task_msg.c +++ b/tasks/task_msg.c @@ -26,6 +26,7 @@ void TaskMessaging(const void * argument) xQueueReceive(queRxDataHandle, &slot, osWaitForever); assert_param(slot.len>0 && slot.len<=64); // check the len is within bounds + // We need thr scratch buffer for many unit command handlers TF_Accept(comm, slot.data, slot.len); #if USE_STACK_MONITOR @@ -34,4 +35,4 @@ void TaskMessaging(const void * argument) msgQueHighWaterMark = MAX(msgQueHighWaterMark, count); #endif } -} \ No newline at end of file +} diff --git a/units/digital_in/unit_din.c b/units/digital_in/unit_din.c index f55247e..01c321b 100644 --- a/units/digital_in/unit_din.c +++ b/units/digital_in/unit_din.c @@ -210,7 +210,7 @@ static error_t DI_handleRequest(Unit *unit, TF_ID frame_id, uint8_t command, Pay case CMD_READ:; TRY(UU_DI_Read(unit, &packed)); - PayloadBuilder pb = pb_start((uint8_t*)unit_tmp512, 64, NULL); + PayloadBuilder pb = pb_start((uint8_t*)unit_tmp512, UNIT_TMP_LEN, NULL); pb_u16(&pb, packed); com_respond_buf(frame_id, MSG_SUCCESS, (uint8_t *) unit_tmp512, pb_length(&pb)); return E_SUCCESS; diff --git a/units/i2c/unit_i2c.c b/units/i2c/unit_i2c.c index 734dc6e..796691a 100644 --- a/units/i2c/unit_i2c.c +++ b/units/i2c/unit_i2c.c @@ -380,7 +380,7 @@ error_t UU_I2C_WriteReg(Unit *unit, uint16_t addr, uint8_t regnum, const uint8_t CHECK_TYPE(unit, &UNIT_I2C); // we have to insert the address first - needs a buffer (XXX realistically the buffer needs 1-4 bytes + addr) - PayloadBuilder pb = pb_start((uint8_t*)unit_tmp512, 512, NULL); + PayloadBuilder pb = pb_start((uint8_t*)unit_tmp512, UNIT_TMP_LEN, NULL); pb_u8(&pb, regnum); pb_buf(&pb, bytes, width);