Merge branch 'bugfix/freemodbus_tcp_master_fix_long_frame_buffer_issue' into 'master'

freemodbus: fix tcp master long frame buffer issue

See merge request espressif/esp-idf!11687

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, &reg_info);
     if ((error == ESP_OK) && (cid == reg_info.cid)) {
-        error = mbc_tcp_master_send_request(&request, &param_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*)&param_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, &reg_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*)&param_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, &param_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 {

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();

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)
 

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))) {
+                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 {
+                        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 {
+                    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 {
-                        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));
                 }
                 vTaskDelay(POLL_TIMEOUT_TICS); // timeout between polls
             }

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,