From 0d393558d37fc6396113fcccbdee41098b6e5c50 Mon Sep 17 00:00:00 2001 From: Alex Lisitsyn Date: Fri, 5 Nov 2021 16:23:35 +0000 Subject: [PATCH] freemodbus: fix tcp master long frame buffer issue --- .../modbus_controller/mbc_tcp_master.c | 68 ++++++----- .../tcp_master/port/port_tcp_master.c | 14 +-- .../mb_example_common/include/modbus_params.h | 7 ++ .../tcp/mb_tcp_master/main/tcp_master.c | 113 ++++++++++++------ .../modbus/tcp/mb_tcp_slave/main/tcp_slave.c | 2 +- 5 files changed, 133 insertions(+), 71 deletions(-) diff --git a/components/freemodbus/tcp_master/modbus_controller/mbc_tcp_master.c b/components/freemodbus/tcp_master/modbus_controller/mbc_tcp_master.c index 1b7f3eb92a..132c148f79 100644 --- a/components/freemodbus/tcp_master/modbus_controller/mbc_tcp_master.c +++ b/components/freemodbus/tcp_master/modbus_controller/mbc_tcp_master.c @@ -1,16 +1,7 @@ -/* Copyright 2018 Espressif Systems (Shanghai) PTE LTD +/* + * SPDX-FileCopyrightText: 2016-2021 Espressif Systems (Shanghai) CO LTD * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * SPDX-License-Identifier: Apache-2.0 */ // mbc_tcp_master.c @@ -135,7 +126,6 @@ static esp_err_t mbc_tcp_master_start(void) MB_MASTER_CHECK((start), ESP_ERR_INVALID_STATE, "mb stack could not connect to slaves for %d seconds.", CONFIG_FMB_TCP_CONNECTION_TOUT_SEC); - return ESP_OK; } @@ -153,7 +143,7 @@ static esp_err_t mbc_tcp_master_destroy(void) mb_error = eMBMasterClose(); MB_MASTER_CHECK((mb_error == MB_ENOERR), ESP_ERR_INVALID_STATE, "mb stack close failure returned (0x%x).", (uint32_t)mb_error); - // Stop polling by clearing correspondent bit in the event group +// 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); @@ -184,9 +174,10 @@ static esp_err_t mbc_tcp_master_set_descriptor(const mb_parameter_descriptor_t* { MB_MASTER_CHECK((comm_ip_table[reg_ptr->mb_slave_addr - 1] != NULL), ESP_ERR_INVALID_ARG, "mb ip table address is incorrect."); // Below is the code to check consistency of the table format and required fields. - MB_MASTER_CHECK((reg_ptr->cid == counter), ESP_ERR_INVALID_ARG, "mb descriptor cid field is incorrect."); - MB_MASTER_CHECK((reg_ptr->param_key != NULL), ESP_ERR_INVALID_ARG, "mb descriptor param key is incorrect."); - MB_MASTER_CHECK((reg_ptr->mb_size > 0), ESP_ERR_INVALID_ARG, "mb descriptor param size is incorrect."); + MB_MASTER_CHECK((reg_ptr->cid == counter), ESP_ERR_INVALID_ARG, "cid: %d, mb descriptor cid field is incorrect.", reg_ptr->cid); + MB_MASTER_CHECK((reg_ptr->param_key != NULL), ESP_ERR_INVALID_ARG, "cid: %d, mb descriptor param key is incorrect.", reg_ptr->cid); + MB_MASTER_CHECK((reg_ptr->mb_size > 0), ESP_ERR_INVALID_ARG, "cid: %d, mb descriptor param size is incorrect.", reg_ptr->cid); + MB_MASTER_CHECK(((reg_ptr->mb_size << 1) >= reg_ptr->param_size), ESP_ERR_INVALID_ARG, "cid: %d, mb descriptor param size is incorrect.", reg_ptr->cid); } mbm_opts->mbm_param_descriptor_table = descriptor; mbm_opts->mbm_param_descriptor_size = num_elements; @@ -420,27 +411,39 @@ static esp_err_t mbc_tcp_master_get_parameter(uint16_t cid, char* name, uint8_t* { MB_MASTER_CHECK((name != NULL), ESP_ERR_INVALID_ARG, "mb incorrect descriptor."); MB_MASTER_CHECK((type != NULL), ESP_ERR_INVALID_ARG, "type pointer is incorrect."); + MB_MASTER_CHECK((value != NULL), ESP_ERR_INVALID_ARG, "value pointer is incorrect."); esp_err_t error = ESP_ERR_INVALID_RESPONSE; mb_param_request_t request ; mb_parameter_descriptor_t reg_info = { 0 }; - uint8_t param_buffer[PARAM_MAX_SIZE] = { 0 }; + uint8_t* pdata = NULL; error = mbc_tcp_master_set_request(name, MB_PARAM_READ, &request, ®_info); if ((error == ESP_OK) && (cid == reg_info.cid)) { - error = mbc_tcp_master_send_request(&request, ¶m_buffer[0]); + // alloc buffer to store parameter data + pdata = calloc(1, (reg_info.mb_size << 1)); + if (!pdata) { + return ESP_ERR_INVALID_STATE; + } + error = mbc_tcp_master_send_request(&request, pdata); if (error == ESP_OK) { // If data pointer is NULL then we don't need to set value (it is still in the cache of cid) if (value != NULL) { - error = mbc_tcp_master_set_param_data((void*)value, (void*)¶m_buffer[0], + error = mbc_tcp_master_set_param_data((void*)value, (void*)pdata, reg_info.param_type, reg_info.param_size); - MB_MASTER_CHECK((error == ESP_OK), ESP_ERR_INVALID_STATE, "fail to set parameter data."); + if (error != ESP_OK) { + ESP_LOGE(MB_MASTER_TAG, "fail to set parameter data."); + error = ESP_ERR_INVALID_STATE; + } else { + ESP_LOGD(MB_MASTER_TAG, "%s: Good response for get cid(%u) = %s", + __FUNCTION__, (unsigned)reg_info.cid, (char*)esp_err_to_name(error)); + } } - ESP_LOGD(MB_MASTER_TAG, "%s: Good response for get cid(%u) = %s", - __FUNCTION__, (int)reg_info.cid, (char*)esp_err_to_name(error)); } else { ESP_LOGD(MB_MASTER_TAG, "%s: Bad response to get cid(%u) = %s", __FUNCTION__, reg_info.cid, (char*)esp_err_to_name(error)); + error = ESP_ERR_INVALID_RESPONSE; } + free(pdata); // Set the type of parameter found in the table *type = reg_info.param_type; } else { @@ -461,23 +464,32 @@ static esp_err_t mbc_tcp_master_set_parameter(uint16_t cid, char* name, uint8_t* esp_err_t error = ESP_ERR_INVALID_RESPONSE; mb_param_request_t request ; mb_parameter_descriptor_t reg_info = { 0 }; - uint8_t param_buffer[PARAM_MAX_SIZE] = { 0 }; + uint8_t* pdata = NULL; error = mbc_tcp_master_set_request(name, MB_PARAM_WRITE, &request, ®_info); if ((error == ESP_OK) && (cid == reg_info.cid)) { + pdata = calloc(1, (reg_info.mb_size << 1)); // alloc parameter buffer + if (!pdata) { + return ESP_ERR_INVALID_STATE; + } // Transfer value of characteristic into parameter buffer - error = mbc_tcp_master_set_param_data((void*)¶m_buffer[0], (void*)value, + error = mbc_tcp_master_set_param_data((void*)pdata, (void*)value, reg_info.param_type, reg_info.param_size); - MB_MASTER_CHECK((error == ESP_OK), ESP_ERR_INVALID_STATE, "failure to set parameter data."); + if (error != ESP_OK) { + ESP_LOGE(MB_MASTER_TAG, "fail to set parameter data."); + free(pdata); + return ESP_ERR_INVALID_STATE; + } // Send request to write characteristic data - error = mbc_tcp_master_send_request(&request, ¶m_buffer[0]); + error = mbc_tcp_master_send_request(&request, pdata); if (error == ESP_OK) { ESP_LOGD(MB_MASTER_TAG, "%s: Good response for set cid(%u) = %s", - __FUNCTION__, (int)reg_info.cid, (char*)esp_err_to_name(error)); + __FUNCTION__, (unsigned)reg_info.cid, (char*)esp_err_to_name(error)); } else { ESP_LOGD(MB_MASTER_TAG, "%s: Bad response to set cid(%u) = %s", __FUNCTION__, reg_info.cid, (char*)esp_err_to_name(error)); } + free(pdata); // Set the type of parameter found in the table *type = reg_info.param_type; } else { diff --git a/components/freemodbus/tcp_master/port/port_tcp_master.c b/components/freemodbus/tcp_master/port/port_tcp_master.c index 2c99c9a1a0..16385eb335 100644 --- a/components/freemodbus/tcp_master/port/port_tcp_master.c +++ b/components/freemodbus/tcp_master/port/port_tcp_master.c @@ -801,30 +801,30 @@ static void vMBTCPPortMasterTask(void *pvParameters) } else { // Check to make sure that active slave data is ready if (FD_ISSET(pxCurrInfo->xSockId, &xReadSet)) { - xErr = ERR_BUF; - for (int retry = 0; (xErr == ERR_BUF) && (retry < MB_TCP_READ_BUF_RETRY_CNT); retry++) { - xErr = vMBTCPPortMasterReadPacket(pxCurrInfo); + int xRet = ERR_BUF; + for (int retry = 0; (xRet == ERR_BUF) && (retry < MB_TCP_READ_BUF_RETRY_CNT); retry++) { + xRet = vMBTCPPortMasterReadPacket(pxCurrInfo); // The error ERR_BUF means received response to previous request // (due to timeout) with the same socket ID and incorrect TID, // then ignore it and try to get next response buffer. } - if (xErr > 0) { + if (xRet > 0) { // Response received correctly, send an event to stack xMBTCPPortMasterFsmSetError(EV_ERROR_INIT, EV_MASTER_FRAME_RECEIVED); ESP_LOGD(MB_TCP_MASTER_PORT_TAG, MB_SLAVE_FMT(", frame received."), pxCurrInfo->xIndex, pxCurrInfo->xSockId, pxCurrInfo->pcIpAddr); - } else if ((xErr == ERR_TIMEOUT) || (xMBTCPPortMasterGetRespTimeLeft(pxCurrInfo) == 0)) { + } else if ((xRet == ERR_TIMEOUT) || (xMBTCPPortMasterGetRespTimeLeft(pxCurrInfo) == 0)) { // Timeout occurred when receiving frame, process respond timeout ESP_LOGD(MB_TCP_MASTER_PORT_TAG, MB_SLAVE_FMT(", frame read timeout."), pxCurrInfo->xIndex, pxCurrInfo->xSockId, pxCurrInfo->pcIpAddr); - } else if (xErr == ERR_BUF) { + } else if (xRet == ERR_BUF) { // After retries a response with incorrect TID received, process failure. xMBTCPPortMasterFsmSetError(EV_ERROR_RECEIVE_DATA, EV_MASTER_ERROR_PROCESS); ESP_LOGD(MB_TCP_MASTER_PORT_TAG, MB_SLAVE_FMT(", frame error."), pxCurrInfo->xIndex, pxCurrInfo->xSockId, pxCurrInfo->pcIpAddr); } else { ESP_LOGE(MB_TCP_MASTER_PORT_TAG, MB_SLAVE_FMT(", critical error=%d."), - pxCurrInfo->xIndex, pxCurrInfo->xSockId, pxCurrInfo->pcIpAddr, xErr); + pxCurrInfo->xIndex, pxCurrInfo->xSockId, pxCurrInfo->pcIpAddr, xRet); // Stop polling process vMBTCPPortMasterStopPoll(); xMBTCPPortMasterCheckShutdown(); diff --git a/examples/protocols/modbus/mb_example_common/include/modbus_params.h b/examples/protocols/modbus/mb_example_common/include/modbus_params.h index 37ee53d84a..6060fa83a8 100644 --- a/examples/protocols/modbus/mb_example_common/include/modbus_params.h +++ b/examples/protocols/modbus/mb_example_common/include/modbus_params.h @@ -1,3 +1,9 @@ +/* + * SPDX-FileCopyrightText: 2016-2021 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ + /*===================================================================================== * Description: * The Modbus parameter structures used to define Modbus instances that @@ -44,6 +50,7 @@ typedef struct float input_data5; float input_data6; float input_data7; + uint16_t data_block1[150]; } input_reg_params_t; #pragma pack(pop) diff --git a/examples/protocols/modbus/tcp/mb_tcp_master/main/tcp_master.c b/examples/protocols/modbus/tcp/mb_tcp_master/main/tcp_master.c index e2bd6e19a5..688acf6260 100644 --- a/examples/protocols/modbus/tcp/mb_tcp_master/main/tcp_master.c +++ b/examples/protocols/modbus/tcp/mb_tcp_master/main/tcp_master.c @@ -81,6 +81,7 @@ enum { CID_HOLD_DATA_1, CID_INP_DATA_2, CID_HOLD_DATA_2, + CID_HOLD_TEST_REG, CID_RELAY_P1, CID_RELAY_P2, CID_COUNT @@ -109,10 +110,12 @@ const mb_parameter_descriptor_t device_parameters[] = { INPUT_OFFSET(input_data2), PARAM_TYPE_FLOAT, 4, OPTS( -40, 100, 1 ), PAR_PERMS_READ_WRITE_TRIGGER }, { CID_HOLD_DATA_2, STR("Humidity_3"), STR("%rH"), MB_DEVICE_ADDR1, MB_PARAM_HOLDING, 4, 2, HOLD_OFFSET(holding_data2), PARAM_TYPE_FLOAT, 4, OPTS( 0, 100, 1 ), PAR_PERMS_READ_WRITE_TRIGGER }, + { CID_HOLD_TEST_REG, STR("Test_regs"), STR("__"), MB_DEVICE_ADDR1, MB_PARAM_HOLDING, 8, 100, + HOLD_OFFSET(test_regs), PARAM_TYPE_ASCII, 200, OPTS( 0, 100, 1 ), PAR_PERMS_READ_WRITE_TRIGGER }, { CID_RELAY_P1, STR("RelayP1"), STR("on/off"), MB_DEVICE_ADDR1, MB_PARAM_COIL, 0, 8, - COIL_OFFSET(coils_port0), PARAM_TYPE_U16, 2, OPTS( BIT1, 0, 0 ), PAR_PERMS_READ_WRITE_TRIGGER }, + COIL_OFFSET(coils_port0), PARAM_TYPE_U16, 1, OPTS( BIT1, 0, 0 ), PAR_PERMS_READ_WRITE_TRIGGER }, { CID_RELAY_P2, STR("RelayP2"), STR("on/off"), MB_DEVICE_ADDR1, MB_PARAM_COIL, 8, 8, - COIL_OFFSET(coils_port1), PARAM_TYPE_U16, 2, OPTS( BIT0, 0, 0 ), PAR_PERMS_READ_WRITE_TRIGGER } + COIL_OFFSET(coils_port1), PARAM_TYPE_U16, 1, OPTS( BIT0, 0, 0 ), PAR_PERMS_READ_WRITE_TRIGGER } }; // Calculate number of parameters in the table @@ -437,43 +440,83 @@ static void master_operation_func(void *arg) void* temp_data_ptr = master_get_param_data(param_descriptor); assert(temp_data_ptr); uint8_t type = 0; - err = mbc_master_get_parameter(cid, (char*)param_descriptor->param_key, - (uint8_t*)&value, &type); - if (err == ESP_OK) { - *(float*)temp_data_ptr = value; - if ((param_descriptor->mb_param_type == MB_PARAM_HOLDING) || - (param_descriptor->mb_param_type == MB_PARAM_INPUT)) { - ESP_LOGI(MASTER_TAG, "Characteristic #%d %s (%s) value = %f (0x%x) read successful.", - param_descriptor->cid, - (char*)param_descriptor->param_key, - (char*)param_descriptor->param_units, - value, - *(uint32_t*)temp_data_ptr); - if (((value > param_descriptor->param_opts.max) || - (value < param_descriptor->param_opts.min))) { - alarm_state = true; - break; + if ((param_descriptor->param_type == PARAM_TYPE_ASCII) && + (param_descriptor->cid == CID_HOLD_TEST_REG)) { + // Check for long array of registers of type PARAM_TYPE_ASCII + err = mbc_master_get_parameter(cid, (char*)param_descriptor->param_key, + (uint8_t*)temp_data_ptr, &type); + if (err == ESP_OK) { + ESP_LOGI(MASTER_TAG, "Characteristic #%d %s (%s) value = (0x%08x) read successful.", + param_descriptor->cid, + (char*)param_descriptor->param_key, + (char*)param_descriptor->param_units, + *(uint32_t*)temp_data_ptr); + // Initialize data of test array and write to slave + if (*(uint32_t*)temp_data_ptr != 0xAAAAAAAA) { + memset((void*)temp_data_ptr, 0xAA, param_descriptor->param_size); + *(uint32_t*)temp_data_ptr = 0xAAAAAAAA; + err = mbc_master_set_parameter(cid, (char*)param_descriptor->param_key, + (uint8_t*)temp_data_ptr, &type); + if (err == ESP_OK) { + ESP_LOGI(MASTER_TAG, "Characteristic #%d %s (%s) value = (0x%08x), write successful.", + param_descriptor->cid, + (char*)param_descriptor->param_key, + (char*)param_descriptor->param_units, + *(uint32_t*)temp_data_ptr); + } else { + ESP_LOGE(MASTER_TAG, "Characteristic #%d (%s) write fail, err = 0x%x (%s).", + param_descriptor->cid, + (char*)param_descriptor->param_key, + (int)err, + (char*)esp_err_to_name(err)); + } } } else { - uint16_t state = *(uint16_t*)temp_data_ptr; - const char* rw_str = (state & param_descriptor->param_opts.opt1) ? "ON" : "OFF"; - ESP_LOGI(MASTER_TAG, "Characteristic #%d %s (%s) value = %s (0x%x) read successful.", - param_descriptor->cid, - (char*)param_descriptor->param_key, - (char*)param_descriptor->param_units, - (const char*)rw_str, - *(uint16_t*)temp_data_ptr); - if (state & param_descriptor->param_opts.opt1) { - alarm_state = true; - break; - } + ESP_LOGE(MASTER_TAG, "Characteristic #%d (%s) read fail, err = 0x%x (%s).", + param_descriptor->cid, + (char*)param_descriptor->param_key, + (int)err, + (char*)esp_err_to_name(err)); } } else { - ESP_LOGE(MASTER_TAG, "Characteristic #%d (%s) read fail, err = %d (%s).", - param_descriptor->cid, - (char*)param_descriptor->param_key, - (int)err, - (char*)esp_err_to_name(err)); + err = mbc_master_get_parameter(cid, (char*)param_descriptor->param_key, + (uint8_t*)temp_data_ptr, &type); + if (err == ESP_OK) { + if ((param_descriptor->mb_param_type == MB_PARAM_HOLDING) || + (param_descriptor->mb_param_type == MB_PARAM_INPUT)) { + value = *(float*)temp_data_ptr; + ESP_LOGI(MASTER_TAG, "Characteristic #%d %s (%s) value = %f (0x%x) read successful.", + param_descriptor->cid, + (char*)param_descriptor->param_key, + (char*)param_descriptor->param_units, + value, + *(uint32_t*)temp_data_ptr); + if (((value > param_descriptor->param_opts.max) || + (value < param_descriptor->param_opts.min))) { + alarm_state = true; + break; + } + } else { + uint8_t state = *(uint8_t*)temp_data_ptr; + const char* rw_str = (state & param_descriptor->param_opts.opt1) ? "ON" : "OFF"; + ESP_LOGI(MASTER_TAG, "Characteristic #%d %s (%s) value = %s (0x%x) read successful.", + param_descriptor->cid, + (char*)param_descriptor->param_key, + (char*)param_descriptor->param_units, + (const char*)rw_str, + *(uint8_t*)temp_data_ptr); + if (state & param_descriptor->param_opts.opt1) { + alarm_state = true; + break; + } + } + } else { + ESP_LOGE(MASTER_TAG, "Characteristic #%d (%s) read fail, err = 0x%x (%s).", + param_descriptor->cid, + (char*)param_descriptor->param_key, + (int)err, + (char*)esp_err_to_name(err)); + } } vTaskDelay(POLL_TIMEOUT_TICS); // timeout between polls } diff --git a/examples/protocols/modbus/tcp/mb_tcp_slave/main/tcp_slave.c b/examples/protocols/modbus/tcp/mb_tcp_slave/main/tcp_slave.c index f8a920cea0..0394702434 100644 --- a/examples/protocols/modbus/tcp/mb_tcp_slave/main/tcp_slave.c +++ b/examples/protocols/modbus/tcp/mb_tcp_slave/main/tcp_slave.c @@ -329,7 +329,7 @@ static esp_err_t slave_init(mb_communication_info_t* comm_info) reg_area.type = MB_PARAM_HOLDING; // Set type of register area reg_area.start_offset = MB_REG_HOLDING_START_AREA0; // Offset of register area in Modbus protocol reg_area.address = (void*)&holding_reg_params.holding_data0; // Set pointer to storage instance - reg_area.size = sizeof(float) << 2; // Set the size of register storage instance + reg_area.size = (MB_REG_HOLDING_START_AREA1 - MB_REG_HOLDING_START_AREA0) << 1; // Set the size of register storage instance err = mbc_slave_set_descriptor(reg_area); ESP_RETURN_ON_FALSE((err == ESP_OK), ESP_ERR_INVALID_STATE, SLAVE_TAG,