From 8cc415c1706f340e2997270f8a040474681e451e Mon Sep 17 00:00:00 2001 From: Tobias Jahnke Date: Tue, 15 May 2018 16:21:11 +0200 Subject: agl-service-unicens: robustess measure for tx msg - implement asynchronous message transmission via queue - set message buffer to 512 bytes payload consistently - xml parsing robustness improvement - fix error 7 caused by unnecessary gpio port initialization Bug-AGL: SPEC-1177 Change-Id: I8f494288d03d60d7a0a147f0e25ceb3665731782 Signed-off-by: Tobias Jahnke (cherry picked from commit a85efd7f8f33e04c82e44675e126089a9adbd5e3) --- ucs2-interface/ucs-xml/UcsXml.c | 49 ++++++++++++++++----- ucs2-interface/ucs-xml/UcsXml_Private.c | 6 +-- ucs2-interface/ucs_config.h | 17 ++++++- ucs2-interface/ucs_lib_interf.c | 78 +++++++++++++++++++++++---------- ucs2-lib/cfg_agl/ucs_cfg.h | 10 ++--- 5 files changed, 113 insertions(+), 47 deletions(-) diff --git a/ucs2-interface/ucs-xml/UcsXml.c b/ucs2-interface/ucs-xml/UcsXml.c index 2360c7a..7715cd4 100644 --- a/ucs2-interface/ucs-xml/UcsXml.c +++ b/ucs2-interface/ucs-xml/UcsXml.c @@ -42,6 +42,7 @@ #define RETURN_ASSERT(result) { UcsXml_CB_OnError("Assertion in file=%s, line=%d", 2, __FILE__, __LINE__); return result; } #define MISC_HB(value) ((uint8_t)((uint16_t)(value) >> 8)) #define MISC_LB(value) ((uint8_t)((uint16_t)(value) & (uint16_t)0xFF)) +#define ROUTE_AUTO_ID_START (0x8000) struct UcsXmlRoute { @@ -260,6 +261,7 @@ static ParseResult_t ParseScriptPortWrite(xmlNode *act, Ucs_Ns_Script_t *scr, Pr static ParseResult_t ParseScriptPortRead(xmlNode *act, Ucs_Ns_Script_t *scr, PrivateData_t *priv); static ParseResult_t ParseScriptPause(xmlNode *act, Ucs_Ns_Script_t *scr, PrivateData_t *priv); static ParseResult_t ParseRoutes(UcsXmlVal_t *ucs, PrivateData_t *priv); +static bool IsAutoRouteId(uint16_t id, PrivateData_t *priv); /************************************************************************/ /* Public Functions */ @@ -708,7 +710,7 @@ static ParseResult_t ParseAll(xmlNode *tree, UcsXmlVal_t *ucs, PrivateData_t *pr uint32_t nodeCount; xmlNode *sub; ParseResult_t result; - priv->autoRouteId = 0x8000; + priv->autoRouteId = ROUTE_AUTO_ID_START; if (!GetCount(tree, NODE, &nodeCount, true)) RETURN_ASSERT(Parse_XmlError); @@ -1535,9 +1537,19 @@ static ParseResult_t ParseRoutes(UcsXmlVal_t *ucs, PrivateData_t *priv) sourceRoute = priv->pRtLst; while (NULL != sourceRoute) { - if (!sourceRoute->isSource) /*There can be more sinks than sources, so count them*/ + if (sourceRoute->isSource) { - ++routeAmount; + struct UcsXmlRoute *sinkRoute = priv->pRtLst; + while (NULL != sinkRoute) + { + if (sourceRoute != sinkRoute + && !sinkRoute->isSource + && (0 == strncmp(sourceRoute->routeName, sinkRoute->routeName, sizeof(sourceRoute->routeName)))) + { + routeAmount++; + } + sinkRoute = sinkRoute->next; + } } sourceRoute = sourceRoute->next; } @@ -1559,22 +1571,30 @@ static ParseResult_t ParseRoutes(UcsXmlVal_t *ucs, PrivateData_t *priv) && !sinkRoute->isSource && (0 == strncmp(sourceRoute->routeName, sinkRoute->routeName, sizeof(sourceRoute->routeName)))) { - Ucs_Rm_Route_t *route = &ucs->pRoutes[ucs->routesSize++]; + Ucs_Rm_Route_t *route; + if(ucs->routesSize >= routeAmount) + { + RETURN_ASSERT(Parse_MemoryError); + } + route = &ucs->pRoutes[ucs->routesSize++]; route->source_endpoint_ptr = sourceRoute->ep; route->sink_endpoint_ptr = sinkRoute->ep; - route->active = sinkRoute->isActive; - route->route_id = sinkRoute->routeId; + if (!IsAutoRouteId(sourceRoute->routeId, priv)) + { + route->active = sourceRoute->isActive; + route->route_id = sourceRoute->routeId; + } + else + { + route->active = sinkRoute->isActive; + route->route_id = sinkRoute->routeId; + } } sinkRoute = sinkRoute->next; } } sourceRoute = sourceRoute->next; } - if (routeAmount != ucs->routesSize) - { - UcsXml_CB_OnError("At least one sink (num=%d) is not connected, because of wrong Route name!", 2, (routeAmount - ucs->routesSize)); - RETURN_ASSERT(Parse_XmlError); - } #ifdef DEBUG /* Third perform checks when running in debug mode*/ @@ -1610,3 +1630,10 @@ static ParseResult_t ParseRoutes(UcsXmlVal_t *ucs, PrivateData_t *priv) #endif return Parse_Success; } + +static bool IsAutoRouteId(uint16_t id, PrivateData_t *priv) +{ + assert(NULL != priv); + return (id >= ROUTE_AUTO_ID_START && id <= priv->autoRouteId); +} + diff --git a/ucs2-interface/ucs-xml/UcsXml_Private.c b/ucs2-interface/ucs-xml/UcsXml_Private.c index 743598f..4fb4e0c 100644 --- a/ucs2-interface/ucs-xml/UcsXml_Private.c +++ b/ucs2-interface/ucs-xml/UcsXml_Private.c @@ -65,10 +65,6 @@ static const char* I2S_PIN_SRXB1 = "SRXB1"; static const char* MUTE_OFF = "NoMuting"; static const char* MUTE_SIGNAL = "MuteSignal"; -/* -static const char* VAL_TRUE = "true"; -static const char* VAL_FALSE = "false"; - */ #define ASSERT_FALSE(func, par) { UcsXml_CB_OnError("Parameter error in attribute=%s value=%s, file=%s, line=%d", 4, func, par, __FILE__, __LINE__); return false; } #define CHECK_POINTER(PTR) if (NULL == PTR) { ASSERT_FALSE(PTR, "NULL pointer"); } @@ -508,4 +504,4 @@ bool GetAvpCon(Ucs_Xrm_AvpCon_t **avpCon, struct AvpConParameters *param) con->isoc_packet_size = UCS_ISOC_PCKT_SIZE_188; } return true; -} \ No newline at end of file +} diff --git a/ucs2-interface/ucs_config.h b/ucs2-interface/ucs_config.h index 1f1ecdd..b23d169 100644 --- a/ucs2-interface/ucs_config.h +++ b/ucs2-interface/ucs_config.h @@ -84,7 +84,8 @@ typedef enum UnicensCmd_NsRun, UnicensCmd_GpioCreatePort, UnicensCmd_GpioWritePort, - UnicensCmd_I2CWrite + UnicensCmd_I2CWrite, + UnicensCmd_SendAmsMessage } UnicensCmd_t; /** @@ -150,7 +151,18 @@ typedef struct } UnicensCmdI2CWrite_t; /** - * \brief Internal struct for Unicens Integration + * \brief Internal struct for UNICENS Integration + */ +typedef struct +{ + uint16_t msgId; + uint16_t targetAddress; + uint8_t pPayload[UCS_AMS_SIZE_TX_MSG]; + uint32_t payloadLen; +} UnicensCmdSendAmsMessage_t; + +/** + * \brief Internal struct for UNICENS Integration */ typedef struct { @@ -163,6 +175,7 @@ typedef struct UnicensCmdGpioCreatePort_t GpioCreatePort; UnicensCmdGpioWritePort_t GpioWritePort; UnicensCmdI2CWrite_t I2CWrite; + UnicensCmdSendAmsMessage_t SendAms; } val; } UnicensCmdEntry_t; diff --git a/ucs2-interface/ucs_lib_interf.c b/ucs2-interface/ucs_lib_interf.c index c851883..f7376c3 100644 --- a/ucs2-interface/ucs_lib_interf.c +++ b/ucs2-interface/ucs_lib_interf.c @@ -73,6 +73,7 @@ static void OnUcsGpioTriggerEventStatus(uint16_t node_address, uint16_t gpio_por uint16_t rising_edges, uint16_t falling_edges, uint16_t levels, void * user_ptr); static void OnUcsI2CWrite(uint16_t node_address, uint16_t i2c_port_handle, uint8_t i2c_slave_address, uint8_t data_len, Ucs_I2c_Result_t result, void *user_ptr); +static void OnUcsAmsWrite(Ucs_AmsTx_Msg_t* msg_ptr, Ucs_AmsTx_Result_t result, Ucs_AmsTx_Info_t info, void *user_ptr); /************************************************************************/ /* Public Function Implementations */ @@ -231,6 +232,37 @@ void UCSI_Service(UCSI_Data_t *my) e->val.I2CWrite.result_fptr(NULL /*processing error*/, e->val.I2CWrite.request_ptr); } break; + case UnicensCmd_SendAmsMessage: + { + Ucs_AmsTx_Msg_t *msg; + msg = Ucs_AmsTx_AllocMsg(my->unicens, e->val.SendAms.payloadLen); + if (NULL == msg) + { + /* Try again later */ + popEntry = false; + break; + } + if (0 != e->val.SendAms.payloadLen) + { + assert(NULL != msg->data_ptr); + memcpy(msg->data_ptr, e->val.SendAms.pPayload, e->val.SendAms.payloadLen); + } + msg->custom_info_ptr = NULL; + msg->data_size = e->val.SendAms.payloadLen; + msg->destination_address = e->val.SendAms.targetAddress; + msg->llrbc = 10; + msg->msg_id = e->val.SendAms.msgId; + if (UCS_RET_SUCCESS == Ucs_AmsTx_SendMsg(my->unicens, msg, OnUcsAmsWrite)) + { + popEntry = false; + } + else + { + Ucs_AmsTx_FreeUnusedMsg(my->unicens, msg); + UCSI_CB_OnUserMessage(my->tag, true, "Ucs_AmsTx_SendMsg failed", 0); + } + break; + } default: assert(false); break; @@ -251,26 +283,20 @@ void UCSI_Timeout(UCSI_Data_t *my) bool UCSI_SendAmsMessage(UCSI_Data_t *my, uint16_t msgId, uint16_t targetAddress, uint8_t *pPayload, uint32_t payloadLen) { - Ucs_AmsTx_Msg_t *msg; - Ucs_Return_t result; + UnicensCmdEntry_t entry; assert(MAGIC == my->magic); - if (NULL == my->unicens) return false; - msg = Ucs_AmsTx_AllocMsg(my->unicens, payloadLen); - if (NULL == msg) return false; - if (0 != payloadLen) + if (NULL == my) return false; + if (payloadLen > UCS_AMS_SIZE_TX_MSG) { - assert(NULL != msg->data_ptr); - memcpy(msg->data_ptr, pPayload, payloadLen); + UCSI_CB_OnUserMessage(my->tag, true, "SendAms was called with payload length=%d, allowed is=%d", 2, payloadLen, UCS_AMS_SIZE_TX_MSG); + return false; } - msg->custom_info_ptr = NULL; - msg->data_size = payloadLen; - msg->destination_address = targetAddress; - msg->llrbc = 10; - msg->msg_id = msgId; - result = Ucs_AmsTx_SendMsg(my->unicens, msg, NULL); - if (UCS_RET_SUCCESS != result) - Ucs_AmsTx_FreeUnusedMsg(my->unicens, msg); - return UCS_RET_SUCCESS == result; + entry.cmd = UnicensCmd_SendAmsMessage; + entry.val.SendAms.msgId = msgId; + entry.val.SendAms.targetAddress = targetAddress; + entry.val.SendAms.payloadLen = payloadLen; + memcpy(entry.val.SendAms.pPayload, pPayload, payloadLen); + return EnqueueCommand(my, &entry); } bool UCSI_GetAmsMessage(UCSI_Data_t *my, uint16_t *pMsgId, uint16_t *pSourceAddress, uint8_t **pPayload, uint32_t *pPayloadLen) @@ -765,11 +791,6 @@ static void OnUcsMgrReport(Ucs_MgrReport_t code, uint16_t node_address, Ucs_Rm_N { UnicensCmdEntry_t e; UCSI_CB_OnUserMessage(my->tag, false, "Node=%X: Available", 1, node_address); - /* Enable usage of remote GPIO ports */ - e.cmd = UnicensCmd_GpioCreatePort; - e.val.GpioCreatePort.destination = node_address; - e.val.GpioCreatePort.debounceTime = 20; - EnqueueCommand(my, &e); /* Execute scripts, if there are any */ if (node_ptr && node_ptr->script_list_ptr && node_ptr->script_list_size) { @@ -848,6 +869,15 @@ static void OnUcsI2CWrite(uint16_t node_address, uint16_t i2c_port_handle, UCSI_CB_OnUserMessage(my->tag, true, "Remote I2C Write to node=0x%X failed", 1, node_address); } +static void OnUcsAmsWrite(Ucs_AmsTx_Msg_t* msg_ptr, Ucs_AmsTx_Result_t result, Ucs_AmsTx_Info_t info, void *user_ptr) +{ + UCSI_Data_t *my = (UCSI_Data_t *)user_ptr; + assert(MAGIC == my->magic); + OnCommandExecuted(my, UnicensCmd_SendAmsMessage); + if (UCS_AMSTX_RES_SUCCESS != result) + UCSI_CB_OnUserMessage(my->tag, true, "SendAms failed with result=0x%x, info=0x%X", 2, result, info); +} + /************************************************************************/ /* Debug Message output from UNICENS stack: */ /************************************************************************/ @@ -866,7 +896,7 @@ void App_TraceError(void *ucs_user_ptr, const char module_str[], const char entr tag = my->tag; } va_start(argptr, vargs_cnt); - vsprintf(outbuf, entry_str, argptr); + vsnprintf(outbuf, sizeof(outbuf), entry_str, argptr); va_end(argptr); UCSI_CB_OnUserMessage(tag, true, "Error | %s | %s", 2, module_str, outbuf); } @@ -883,7 +913,7 @@ void App_TraceInfo(void *ucs_user_ptr, const char module_str[], const char entry tag = my->tag; } va_start(argptr, vargs_cnt); - vsprintf(outbuf, entry_str, argptr); + vsnprintf(outbuf, sizeof(outbuf), entry_str, argptr); va_end(argptr); UCSI_CB_OnUserMessage(tag, false, "Info | %s | %s", 2, module_str, outbuf); } diff --git a/ucs2-lib/cfg_agl/ucs_cfg.h b/ucs2-lib/cfg_agl/ucs_cfg.h index f0ccdf2..a7d8862 100644 --- a/ucs2-lib/cfg_agl/ucs_cfg.h +++ b/ucs2-lib/cfg_agl/ucs_cfg.h @@ -64,7 +64,7 @@ extern "C" /*------------------------------------------------------------------------------------------------*/ /* Application Messages */ /*------------------------------------------------------------------------------------------------*/ -/* Defines the number of reserved Rx message objects. +/* Defines the number of reserved Rx message objects. * Valid values: 5..255. Default value: 20. */ #define UCS_AMS_NUM_RX_MSGS 20 @@ -72,7 +72,7 @@ extern "C" /* Defines the payload size in bytes which is available for every Rx message object. * Valid values: 45..65535. Default value: 45. */ -#define UCS_AMS_SIZE_RX_MSG 1024 +#define UCS_AMS_SIZE_RX_MSG 512 /* Defines the number of reserved Tx message objects. * Valid values: 5..255. Default value: 20. @@ -87,7 +87,7 @@ extern "C" /*------------------------------------------------------------------------------------------------*/ /* Memory Optimization */ /*------------------------------------------------------------------------------------------------*/ -/* Define the following macros to reduces the RAM and ROM size of the UNICENS software by disabling +/* Define the following macros to reduces the RAM and ROM size of the UNICENS software by disabling * certain features. If this macro is defined the following changes apply: * - Reduction of low-level buffers * - AMS does not support segmentation (payload > 45 bytes) @@ -97,8 +97,8 @@ extern "C" /*------------------------------------------------------------------------------------------------*/ /* Tracing & Debugging */ /*------------------------------------------------------------------------------------------------*/ -/* Define the following macros to map info and error trace output to user defined functions. - * The purpose of these functions is debugging. It is not recommended to define these functions +/* Define the following macros to map info and error trace output to user defined functions. + * The purpose of these functions is debugging. It is not recommended to define these functions * in a production system. */ #define UCS_TR_ERROR App_TraceError -- cgit 1.2.3-korg