Skip to content

Commit

Permalink
Merge branch 'bugfix/fix_tcp_slave_start_destroy_sequence' into 'master'
Browse files Browse the repository at this point in the history 
modbus tcp master, slave fix start - destroy sequence

See merge request idf/esp-modbus!43
  • Loading branch information
@alisitsyn
alisitsyn committed Sep 29, 2023
2 parents 567fe19 + 401dbc8 commit aa93eec
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 107 deletions.
8 changes: 8 additions & 0 deletions freemodbus/port/port.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,14 @@
return ret_val; \
}

// Macro to check if stack shutdown event is active
#define TCP_PORT_CHECK_SHDN(sema_ptr, callback_func) do { \
if (sema_ptr) { \
ESP_LOGD(TAG, "Shutdown stack from %s(%d)", __func__, __LINE__); \
callback_func(); \
} \
} while(0)

#ifdef __cplusplus
PR_BEGIN_EXTERN_C
#endif /* __cplusplus */
Expand Down
17 changes: 10 additions & 7 deletions freemodbus/tcp_master/modbus_controller/mbc_tcp_master.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,25 +181,28 @@ static esp_err_t mbc_tcp_master_start(void)
return ESP_OK;
}

// Modbus controller destroy function
static esp_err_t mbc_tcp_master_destroy(void)
{
MB_MASTER_ASSERT(mbm_interface_ptr != NULL);
mb_master_options_t* mbm_opts = &mbm_interface_ptr->opts;
MB_MASTER_CHECK((mbm_opts != NULL), ESP_ERR_INVALID_ARG, "mb incorrect options pointer.");
eMBErrorCode mb_error = MB_ENOERR;

// Disable and then destroy the Modbus stack
// Stop polling by clearing correspondent bit in the event group
xEventGroupClearBits(mbm_opts->mbm_event_group,
(EventBits_t)MB_EVENT_STACK_STARTED);

// Disable and then destroy the Modbus port
mb_error = eMBMasterDisable();
MB_MASTER_CHECK((mb_error == MB_ENOERR), ESP_ERR_INVALID_STATE, "mb stack disable failure.");

(void)vTaskDelete(mbm_opts->mbm_task_handle);
mbm_opts->mbm_task_handle = NULL;

mb_error = eMBMasterClose();
MB_MASTER_CHECK((mb_error == MB_ENOERR), ESP_ERR_INVALID_STATE,
"mb stack close failure returned (0x%x).", (int)mb_error);
// Stop polling by clearing correspondent bit in the event group
xEventGroupClearBits(mbm_opts->mbm_event_group,
(EventBits_t)MB_EVENT_STACK_STARTED);
(void)vTaskDelete(mbm_opts->mbm_task_handle);
mbm_opts->mbm_task_handle = NULL;

(void)vEventGroupDelete(mbm_opts->mbm_event_group);
mbm_opts->mbm_event_group = NULL;
mbc_tcp_master_free_slave_list();
Expand Down
77 changes: 38 additions & 39 deletions freemodbus/tcp_master/port/port_tcp_master.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
#define MB_EVENT_REQ_ERR_MASK ( EV_MASTER_PROCESS_SUCCESS )

#define MB_EVENT_WAIT_TOUT_MS ( 3000 )
#define MB_SHDN_WAIT_TOUT_MS ( 5000 )

#define MB_TCP_READ_TICK_MS ( 1 )
#define MB_TCP_READ_BUF_RETRY_CNT ( 4 )
Expand All @@ -80,7 +81,7 @@ void vMBPortEventClose(void);
static const char *TAG = "MB_TCP_MASTER_PORT";
static MbPortConfig_t xMbPortConfig;
static EventGroupHandle_t xMasterEventHandle = NULL;
static SemaphoreHandle_t xShutdownSemaphore = NULL;
static SemaphoreHandle_t xShutdownSema = NULL;
static EventBits_t xMasterEvent = 0;

/* ----------------------- Static functions ---------------------------------*/
Expand Down Expand Up @@ -129,7 +130,7 @@ xMBMasterTCPPortInit( USHORT usTCPPort )

// Create task for packet processing
BaseType_t xErr = xTaskCreatePinnedToCore(vMBTCPPortMasterTask,
"tcp_master_task",
"mbm_port_tcp_task",
MB_TCP_STACK_SIZE,
NULL,
MB_TCP_TASK_PRIO,
Expand All @@ -143,8 +144,6 @@ xMBMasterTCPPortInit( USHORT usTCPPort )
ESP_LOGI(TAG, "TCP master stack initialized.");
bOkay = TRUE;
}

vTaskSuspend(xMbPortConfig.xMbTcpTaskHandle);
return bOkay;
}

Expand Down Expand Up @@ -217,15 +216,6 @@ static void vMBTCPPortMasterMStoTimeVal(USHORT usTimeoutMs, struct timeval *tv)
tv->tv_usec = (usTimeoutMs - (tv->tv_sec * 1000)) * 1000;
}

static void xMBTCPPortMasterCheckShutdown(void)
{
// First check if the task is not flagged for shutdown
if (xShutdownSemaphore) {
xSemaphoreGive(xShutdownSemaphore);
vTaskDelete(NULL);
}
}

static BOOL xMBTCPPortMasterCloseConnection(MbSlaveInfo_t *pxInfo)
{
if (!pxInfo) {
Expand All @@ -243,6 +233,26 @@ static BOOL xMBTCPPortMasterCloseConnection(MbSlaveInfo_t *pxInfo)
return TRUE;
}

static void xMBTCPPortMasterShutdown(void)
{
xSemaphoreGive(xShutdownSema);
vTaskDelete(NULL);
xMbPortConfig.xMbTcpTaskHandle = NULL;

for (USHORT ucCnt = 0; ucCnt < MB_TCP_PORT_MAX_CONN; ucCnt++) {
MbSlaveInfo_t* pxInfo = xMbPortConfig.pxMbSlaveInfo[ucCnt];
if (pxInfo) {
xMBTCPPortMasterCloseConnection(pxInfo);
if (pxInfo->pucRcvBuf) {
free(pxInfo->pucRcvBuf);
}
free(pxInfo);
xMbPortConfig.pxMbSlaveInfo[ucCnt] = NULL;
}
}
free(xMbPortConfig.pxMbSlaveInfo);
}

void vMBTCPPortMasterSetNetOpt(void *pvNetIf, eMBPortIpVer xIpVersion, eMBPortProto xProto)
{
xMbPortConfig.pvNetIface = pvNetIf;
Expand Down Expand Up @@ -298,7 +308,7 @@ static int xMBTCPPortMasterGetBuf(MbSlaveInfo_t *pxInfo, UCHAR *pucDstBuf, USHOR

// Receive data from connected client
while (usBytesLeft > 0) {
xMBTCPPortMasterCheckShutdown();
TCP_PORT_CHECK_SHDN(xShutdownSema, xMBTCPPortMasterShutdown);
xLength = recv(pxInfo->xSockId, pucBuf, usBytesLeft, 0);
if (xLength < 0) {
if (errno == EAGAIN) {
Expand Down Expand Up @@ -678,7 +688,7 @@ static void vMBTCPPortMasterTask(void *pvParameters)
while (1) {
MbSlaveAddrInfo_t xSlaveAddrInfo = { 0 };
BaseType_t xStatus = xQueueReceive(xMbPortConfig.xConnectQueue, (void*)&xSlaveAddrInfo, pdMS_TO_TICKS(MB_EVENT_WAIT_TOUT_MS));
xMBTCPPortMasterCheckShutdown();
TCP_PORT_CHECK_SHDN(xShutdownSema, xMBTCPPortMasterShutdown);
if (xStatus != pdTRUE) {
ESP_LOGE(TAG, "Fail to register slave IP.");
} else {
Expand Down Expand Up @@ -785,7 +795,7 @@ static void vMBTCPPortMasterTask(void *pvParameters)
if (pxInfo) {
pxInfo->xError = xErr;
}
xMBTCPPortMasterCheckShutdown();
TCP_PORT_CHECK_SHDN(xShutdownSema, xMBTCPPortMasterShutdown);
}
}
ESP_LOGI(TAG, "Connected %u slaves, start polling...", (unsigned)usSlaveConnCnt);
Expand All @@ -797,6 +807,7 @@ static void vMBTCPPortMasterTask(void *pvParameters)
xReadSet = xConnSet;
// Check transmission event to clear appropriate bit.
xMBMasterPortFsmWaitConfirmation(EV_MASTER_FRAME_TRANSMIT, pdMS_TO_TICKS(MB_EVENT_WAIT_TOUT_MS));
TCP_PORT_CHECK_SHDN(xShutdownSema, xMBTCPPortMasterShutdown);
// Synchronize state machine with send packet event
if (xMBMasterPortFsmWaitConfirmation(EV_MASTER_FRAME_SENT, pdMS_TO_TICKS(MB_EVENT_WAIT_TOUT_MS))) {
ESP_LOGD(TAG, "FSM Synchronized with sent event.");
Expand All @@ -807,7 +818,7 @@ static void vMBTCPPortMasterTask(void *pvParameters)
ESP_LOGE(TAG, "Incorrect connection options for slave index: %d.",
(int)xMbPortConfig.ucCurSlaveIndex);
vMBTCPPortMasterStopPoll();
xMBTCPPortMasterCheckShutdown();
TCP_PORT_CHECK_SHDN(xShutdownSema, xMBTCPPortMasterShutdown);
break; // incorrect slave descriptor, reconnect.
}
xTime = xMBTCPPortMasterGetRespTimeLeft(pxCurrInfo);
Expand All @@ -823,7 +834,7 @@ static void vMBTCPPortMasterTask(void *pvParameters)
xTime = xMBTCPPortMasterGetRespTimeLeft(pxCurrInfo);
// Wait completion of last transaction
xMBMasterPortFsmWaitConfirmation(MB_EVENT_REQ_DONE_MASK, pdMS_TO_TICKS(xTime + 1));
xMBTCPPortMasterCheckShutdown();
TCP_PORT_CHECK_SHDN(xShutdownSema, xMBTCPPortMasterShutdown);
continue;
} else if (xRes < 0) {
// Select error (slave connection or r/w failure).
Expand All @@ -834,7 +845,7 @@ static void vMBTCPPortMasterTask(void *pvParameters)
xMBMasterPortFsmWaitConfirmation(MB_EVENT_REQ_DONE_MASK, pdMS_TO_TICKS(xTime));
// Stop polling process
vMBTCPPortMasterStopPoll();
xMBTCPPortMasterCheckShutdown();
TCP_PORT_CHECK_SHDN(xShutdownSema, xMBTCPPortMasterShutdown);
// Check disconnected slaves, do not need a result just to print information.
xMBTCPPortMasterCheckConnState(&xConnSet);
break;
Expand Down Expand Up @@ -868,7 +879,7 @@ static void vMBTCPPortMasterTask(void *pvParameters)
(int)pxCurrInfo->xIndex, (int)pxCurrInfo->xSockId, pxCurrInfo->pcIpAddr, (int)xRet);
// Stop polling process
vMBTCPPortMasterStopPoll();
xMBTCPPortMasterCheckShutdown();
TCP_PORT_CHECK_SHDN(xShutdownSema, xMBTCPPortMasterShutdown);
// Check disconnected slaves, do not need a result just to print information.
xMBTCPPortMasterCheckConnState(&xConnSet);
break;
Expand All @@ -885,7 +896,7 @@ static void vMBTCPPortMasterTask(void *pvParameters)
(int)pxCurrInfo->xIndex, (int)pxCurrInfo->xSockId, pxCurrInfo->pcIpAddr, xTime);
}
}
xMBTCPPortMasterCheckShutdown();
TCP_PORT_CHECK_SHDN(xShutdownSema, xMBTCPPortMasterShutdown);
} // while(usMbSlaveInfoCount)
} // while (1)
vTaskDelete(NULL);
Expand All @@ -896,35 +907,23 @@ extern void vMBMasterPortTimerClose(void);

void vMBMasterTCPPortEnable(void)
{
vTaskResume(xMbPortConfig.xMbTcpTaskHandle);

}

void vMBMasterTCPPortDisable(void)
{
// Try to exit the task gracefully, so select could release its internal callbacks
// that were allocated on the stack of the task we're going to delete
xShutdownSemaphore = xSemaphoreCreateBinary();
xShutdownSema = xSemaphoreCreateBinary();
// if no semaphore (alloc issues) or couldn't acquire it, just delete the task
if (xShutdownSemaphore == NULL || xSemaphoreTake(xShutdownSemaphore, pdMS_TO_TICKS(MB_EVENT_WAIT_TOUT_MS)) != pdTRUE) {
if (xShutdownSema == NULL || xSemaphoreTake(xShutdownSema, pdMS_TO_TICKS(MB_SHDN_WAIT_TOUT_MS)) != pdTRUE) {
ESP_LOGW(TAG, "Modbus port task couldn't exit gracefully within timeout -> abruptly deleting the task.");
vTaskDelete(xMbPortConfig.xMbTcpTaskHandle);
}
if (xShutdownSemaphore) {
vSemaphoreDelete(xShutdownSemaphore);
xShutdownSemaphore = NULL;
if (xShutdownSema) {
vSemaphoreDelete(xShutdownSema);
xShutdownSema = NULL;
}
for (USHORT ucCnt = 0; ucCnt < MB_TCP_PORT_MAX_CONN; ucCnt++) {
MbSlaveInfo_t* pxInfo = xMbPortConfig.pxMbSlaveInfo[ucCnt];
if (pxInfo) {
xMBTCPPortMasterCloseConnection(pxInfo);
if (pxInfo->pucRcvBuf) {
free(pxInfo->pucRcvBuf);
}
free(pxInfo);
xMbPortConfig.pxMbSlaveInfo[ucCnt] = NULL;
}
}
free(xMbPortConfig.pxMbSlaveInfo);
}

void vMBMasterTCPPortClose(void)
Expand Down
2 changes: 1 addition & 1 deletion freemodbus/tcp_slave/modbus_controller/mbc_tcp_slave.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ esp_err_t mbc_tcp_slave_create(void** handler)
ESP_ERR_NO_MEM, "mb notify queue creation error.");
// Create Modbus controller task
status = xTaskCreatePinnedToCore((void*)&modbus_tcp_slave_task,
"modbus_tcp_slave_task",
"mbs_port_tcp_task",
MB_CONTROLLER_STACK_SIZE,
NULL,
MB_CONTROLLER_PRIORITY,
Expand Down
Loading

0 comments on commit aa93eec

Please sign in to comment.