Bugfix/master broadcast frame handling

modbus/mb_objects/functions/mbfunccoils_master.c

@@ -3,7 +3,7 @@
  *
  * SPDX-License-Identifier: BSD-3-Clause
  *
- * SPDX-FileContributor: 2020-2025 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileContributor: 2020-2026 Espressif Systems (Shanghai) CO LTD
  */
 /*
  * FreeModbus Library: A portable Modbus implementation for Modbus ASCII/RTU.
@@ -85,12 +85,20 @@ mb_exception_t mb_error_to_exception(mb_err_enum_t error_code);
 mb_err_enum_t mbm_rq_read_coils(mb_base_t *inst, uint8_t snd_addr, uint16_t coil_addr, uint16_t coil_num, uint32_t tout)
 {
     uint8_t *mb_frame_ptr;
+
     if (snd_addr > MB_ADDRESS_MAX) {
         return MB_EINVAL;
     }
+
+    /* The broadcast coils read request is not allowed. */
+    if (!snd_addr) {
+        return MB_ENOREG;
+    }
+
     if (!mb_port_event_res_take(inst->port_obj, tout)) {
         return MB_EBUSY;
     }
+
     inst->get_send_buf(inst, &mb_frame_ptr);
     inst->set_dest_addr(inst, snd_addr);
 
@@ -118,9 +126,12 @@ mb_exception_t mbm_fn_read_coils(mb_base_t *inst, uint8_t *frame_ptr, uint16_t *
     mb_exception_t status = MB_EX_NONE;
     mb_err_enum_t reg_status = MB_EILLFUNC;
 
-    /* If this request is broadcast, and it's read mode. This request don't need execute. */
+    if (!len_buf || !frame_ptr || !inst) {
+        return MB_EINVAL;
+    }
+
     if (inst->transp_obj->frm_is_bcast(inst->transp_obj)) {
-        status = MB_EX_NONE;
+        status = MB_EX_ILLEGAL_DATA_ADDRESS;
     } else if (*len_buf >= MB_PDU_SIZE_MIN + MB_PDU_FUNC_READ_SIZE_MIN) {
         inst->get_send_buf(inst, &mb_frame_ptr);
 
@@ -215,7 +226,12 @@ mb_exception_t mbm_fn_write_coil(mb_base_t *inst, uint8_t *frame_ptr, uint16_t *
     (void)inst;
     (void)frame_ptr;
 
-    if (*len_buf == (MB_PDU_FUNC_WRITE_SIZE + MB_PDU_SIZE_MIN)) {
+    if (!len_buf || !frame_ptr || !inst) {
+        return MB_EINVAL;
+    }
+
+    if (*len_buf == (MB_PDU_FUNC_WRITE_SIZE + MB_PDU_SIZE_MIN)
+            || inst->transp_obj->frm_is_bcast(inst->transp_obj)) {
         status = MB_EX_NONE;
     } else {
         /* Can't be a valid write coil register request because the length
@@ -298,8 +314,13 @@ mb_exception_t mbm_fn_write_multi_coils(mb_base_t *inst, uint8_t *frame_ptr, uin
     uint8_t byte_cnt_verify;
     mb_err_enum_t reg_status = MB_EILLFUNC;
 
-    /* If this request is broadcast, the *len_buf is not need check. */
-    if ((*len_buf == MB_PDU_FUNC_WRITE_MUL_SIZE) || inst->transp_obj->frm_is_bcast(inst->transp_obj)) {
+    if (!len_buf || !frame_ptr || !inst) {
+        return MB_EINVAL;
+    }
+
+    /* If this request is broadcast, do not need to call r/w callback. */
+    if ((*len_buf == MB_PDU_FUNC_WRITE_MUL_SIZE)
+            || inst->transp_obj->frm_is_bcast(inst->transp_obj)) {
         inst->get_send_buf(inst, &mb_frame_ptr);
         reg_address = (uint16_t)(frame_ptr[MB_PDU_FUNC_WRITE_MUL_ADDR_OFF] << 8);
         reg_address |= (uint16_t)(frame_ptr[MB_PDU_FUNC_WRITE_MUL_ADDR_OFF + 1]);

modbus/mb_objects/functions/mbfuncdisc_master.c

@@ -3,7 +3,7 @@
  *
  * SPDX-License-Identifier: BSD-3-Clause
  *
- * SPDX-FileContributor: 2020-2025 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileContributor: 2020-2026 Espressif Systems (Shanghai) CO LTD
  */
 /*
  * FreeModbus Library: A portable Modbus implementation for Modbus ASCII/RTU.
@@ -70,6 +70,12 @@ mb_err_enum_t mbm_rq_read_discrete_inputs(mb_base_t *inst, uint8_t snd_addr, uin
     if (!inst || (snd_addr > MB_ADDRESS_MAX)) {
         return MB_EINVAL;
     }
+
+    /* The broadcast discrete read request is nt supported. */
+    if (!snd_addr) {
+        return MB_ENOREG;
+    }
+
     if (!mb_port_event_res_take(inst->port_obj, tout)) {
         return MB_EBUSY;
     }
@@ -99,8 +105,13 @@ mb_exception_t mbm_fn_read_discrete_inputs(mb_base_t *inst, uint8_t *frame_ptr,
 
     mb_exception_t status = MB_EX_NONE;
     mb_err_enum_t reg_status = MB_EILLFUNC;
+
+    if (!len_buf || !frame_ptr || !inst) {
+        return MB_EINVAL;
+    }
+
     if (inst->transp_obj->frm_is_bcast(inst->transp_obj)) {
-        status = MB_EX_NONE;
+        status = MB_EX_ILLEGAL_DATA_ADDRESS;
     } else if (*len_buf >= MB_PDU_SIZE_MIN + MB_PDU_FUNC_READ_SIZE_MIN) {
         inst->get_send_buf(inst, &mb_frame_ptr);
         reg_address = (uint16_t)(mb_frame_ptr[MB_PDU_REQ_READ_ADDR_OFF] << 8);

modbus/mb_objects/functions/mbfuncholding_master.c

@@ -3,7 +3,7 @@
  *
  * SPDX-License-Identifier: BSD-3-Clause
  *
- * SPDX-FileContributor: 2020-2025 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileContributor: 2020-2026 Espressif Systems (Shanghai) CO LTD
  */
 /*
  * FreeModbus Library: A portable Modbus implementation for Modbus ASCII/RTU.
@@ -86,7 +86,7 @@ mb_exception_t mb_error_to_exception(mb_err_enum_t error_code);
 /**
  * This function will request write holding register.
  *
- * @param snd_addr salve address
+ * @param snd_addr slave address
  * @param reg_addr register start address
  * @param reg_data register data to be written
  * @param timeout timeout (-1 will waiting forever)
@@ -113,7 +113,7 @@ mb_err_enum_t mbm_rq_write_holding_reg(mb_base_t *inst, uint8_t snd_addr, uint16
     mb_frame_ptr[MB_PDU_REQ_WRITE_VALUE_OFF] = reg_data >> 8;
     mb_frame_ptr[MB_PDU_REQ_WRITE_VALUE_OFF + 1] = reg_data;
 
-    inst->set_send_len(inst, MB_PDU_SIZE_MIN + MB_PDU_REQ_READ_SIZE);
+    inst->set_send_len(inst, MB_PDU_SIZE_MIN + MB_PDU_REQ_WRITE_SIZE);
 
     (void)mb_port_event_post(inst->port_obj, EVENT(EV_FRAME_TRANSMIT | EV_TRANS_START));
     return mb_port_event_wait_req_finish(inst->port_obj);
@@ -124,7 +124,14 @@ mb_exception_t mbm_fn_write_holding_reg(mb_base_t *inst, uint8_t *frame_ptr, uin
     mb_exception_t status = MB_EX_NONE;
     mb_err_enum_t reg_status = MB_EILLFUNC;
 
-    if (*len_buf == (MB_PDU_SIZE_MIN + MB_PDU_FUNC_WRITE_SIZE)) {
+    if (!len_buf || !frame_ptr || !inst) {
+        return MB_EINVAL;
+    }
+
+    /* For broadcast request do not check the buffer size. */
+    if (*len_buf == (MB_PDU_SIZE_MIN + MB_PDU_FUNC_WRITE_SIZE)
+            || inst->transp_obj->frm_is_bcast(inst->transp_obj)) {
+        ESP_LOGD(__func__, "Length: %u", *len_buf);
         uint16_t reg_address;
         reg_address = (uint16_t)(frame_ptr[MB_PDU_FUNC_WRITE_ADDR_OFF] << 8);
         reg_address |= (uint16_t)(frame_ptr[MB_PDU_FUNC_WRITE_ADDR_OFF + 1]);
@@ -152,7 +159,7 @@ mb_exception_t mbm_fn_write_holding_reg(mb_base_t *inst, uint8_t *frame_ptr, uin
 /**
  * This function will request write multiple holding register.
  *
- * @param snd_addr salve address
+ * @param snd_addr slave address
  * @param reg_addr register start address
  * @param reg_num register total number
  * @param data_ptr data to be written
@@ -207,8 +214,13 @@ mb_exception_t mbm_fn_write_multi_holding_reg(mb_base_t *inst, uint8_t *frame_pt
     uint16_t byte_count;
     mb_err_enum_t reg_status = MB_EILLFUNC;
 
-    /* If this request is broadcast, the *len_buf is not need check. */
-    if ((*len_buf == MB_PDU_SIZE_MIN + MB_PDU_FUNC_WRITE_MUL_SIZE) || inst->transp_obj->frm_is_bcast(inst->transp_obj)) {
+    if (!len_buf || !frame_ptr || !inst) {
+        return MB_EINVAL;
+    }
+
+    if ((*len_buf == MB_PDU_SIZE_MIN + MB_PDU_FUNC_WRITE_MUL_SIZE)
+            || inst->transp_obj->frm_is_bcast(inst->transp_obj)) {
+        ESP_LOGD(__func__, "Length: %u", *len_buf);
         inst->get_send_buf(inst, &mb_frame_ptr);
         reg_address = (uint16_t)(mb_frame_ptr[MB_PDU_REQ_WRITE_MUL_ADDR_OFF] << 8);
         reg_address |= (uint16_t)(mb_frame_ptr[MB_PDU_REQ_WRITE_MUL_ADDR_OFF + 1]);
@@ -245,7 +257,7 @@ mb_exception_t mbm_fn_write_multi_holding_reg(mb_base_t *inst, uint8_t *frame_pt
 /**
  * This function will request read holding register.
  *
- * @param snd_addr salve address
+ * @param snd_addr slave address
  * @param reg_addr register start address
  * @param reg_num register total number
  * @param timeout timeout (-1 will waiting forever)
@@ -259,6 +271,12 @@ mb_err_enum_t mbm_rq_read_holding_reg(mb_base_t *inst, uint8_t snd_addr, uint16_
     if (!inst || (snd_addr > MB_ADDRESS_MAX)) {
         return MB_EINVAL;
     }
+
+    /* The broadcast read holding registers request is not supported. */
+    if (!snd_addr) {
+        return MB_ENOREG;
+    }
+
     if (!mb_port_event_res_take(inst->port_obj, tout)) {
         return MB_EBUSY;
     }
@@ -287,14 +305,11 @@ mb_exception_t mbm_fn_read_holding_reg(mb_base_t *inst, uint8_t *frame_ptr, uint
     uint16_t reg_count;
     mb_err_enum_t reg_status = MB_EILLFUNC;
 
-    /* If this request is broadcast, and it's read mode. This request don't need execute. */
-    bool is_broadcast = false;
-
-    is_broadcast = inst->transp_obj->frm_is_bcast(inst->transp_obj);
-
-    if (is_broadcast == true) {
-        status = MB_EX_NONE;
-    } else if (*len_buf >= MB_PDU_SIZE_MIN + MB_PDU_FUNC_READ_SIZE_MIN) {
+    ESP_LOGD(__func__, "Buffer length: %u, bcast: %u", *len_buf, (unsigned)inst->transp_obj->frm_is_bcast(inst->transp_obj));
+    ESP_LOG_BUFFER_HEX_LEVEL(__func__, (void *)frame_ptr, *len_buf, ESP_LOG_DEBUG);
+    if (inst->transp_obj->frm_is_bcast(inst->transp_obj)) {
+        status = MB_EX_ILLEGAL_DATA_ADDRESS;
+    } else if ((*len_buf >= MB_PDU_SIZE_MIN + MB_PDU_FUNC_READ_SIZE_MIN)) {
         inst->get_send_buf(inst, &mb_frame_ptr);
         reg_address = (uint16_t)(mb_frame_ptr[MB_PDU_REQ_READ_ADDR_OFF] << 8);
         reg_address |= (uint16_t)(mb_frame_ptr[MB_PDU_REQ_READ_ADDR_OFF + 1]);
@@ -332,7 +347,7 @@ mb_exception_t mbm_fn_read_holding_reg(mb_base_t *inst, uint8_t *frame_ptr, uint
 /**
  * This function will request read and write holding register.
  *
- * @param snd_addr salve address
+ * @param snd_addr slave address
  * @param rd_reg_addr read register start address
  * @param rd_reg_num read register total number
  * @param data_ptr data to be written
@@ -393,10 +408,14 @@ mb_exception_t mbm_fn_rw_multi_holding_regs(mb_base_t *inst, uint8_t *frame_ptr,
     uint16_t reg_rd_address, reg_wr_address;
     uint8_t *mb_frame_ptr;
 
-    /* If this request is broadcast, and it's read mode. This request don't need execute. */
-    if (inst->transp_obj->frm_is_bcast(inst->transp_obj)) {
-        status = MB_EX_NONE;
-    } else if (*len_buf >= MB_PDU_SIZE_MIN + MB_PDU_FUNC_READWRITE_SIZE_MIN) {
+    if (!len_buf || !frame_ptr || !inst) {
+        return MB_EINVAL;
+    }
+
+    /* If this request is broadcast, do not check the length */
+    if ((*len_buf >= MB_PDU_SIZE_MIN + MB_PDU_FUNC_READWRITE_SIZE_MIN)
+            || inst->transp_obj->frm_is_bcast(inst->transp_obj)) {
+        ESP_LOGD(__func__, "Length: %u", *len_buf);
         inst->get_send_buf(inst, &mb_frame_ptr);
         reg_rd_address = (uint16_t)(mb_frame_ptr[MB_PDU_REQ_READWRITE_READ_ADDR_OFF] << 8);
         reg_rd_address |= (uint16_t)(mb_frame_ptr[MB_PDU_REQ_READWRITE_READ_ADDR_OFF + 1]);

modbus/mb_objects/functions/mbfuncinput_master.c

@@ -3,7 +3,7 @@
  *
  * SPDX-License-Identifier: BSD-3-Clause
  *
- * SPDX-FileContributor: 2020-2025 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileContributor: 2020-2026 Espressif Systems (Shanghai) CO LTD
  */
 /*
  * FreeModbus Library: A portable Modbus implementation for Modbus ASCII/RTU.
@@ -73,9 +73,16 @@ mb_err_enum_t mbm_rq_read_inp_reg(mb_base_t *inst, uint8_t snd_addr, uint16_t re
     if (!inst || (snd_addr > MB_ADDRESS_MAX)) {
         return MB_EINVAL;
     }
+
+    /* The broadcast read input request is not supported. */
+    if (!snd_addr) {
+        return MB_ENOREG;
+    }
+
     if (!mb_port_event_res_take(inst->port_obj, tout)) {
         return MB_EBUSY;
     }
+
     inst->get_send_buf(inst, &mb_frame_ptr);
     inst->set_dest_addr(inst, snd_addr);
 
@@ -101,9 +108,12 @@ mb_exception_t mbm_fn_read_inp_reg(mb_base_t *inst, uint8_t *frame_ptr, uint16_t
     uint16_t reg_address;
     uint8_t *mb_frame_ptr;
 
-    /* If this request is broadcast, and it's read mode. This request don't need execute. */
+    if (!len_buf || !frame_ptr || !inst) {
+        return MB_EINVAL;
+    }
+
     if (inst->transp_obj->frm_is_bcast(inst->transp_obj)) {
-        status = MB_EX_NONE;
+        status = MB_EX_ILLEGAL_DATA_ADDRESS;
     } else if (*len_buf >= MB_PDU_SIZE_MIN + MB_PDU_FUNC_READ_SIZE_MIN) {
         inst->get_send_buf(inst, &mb_frame_ptr);
 

modbus/mb_objects/mb_master.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2021-2026 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -486,7 +486,11 @@ static void mbm_set_dest_addr(mb_base_t *inst, uint8_t dest_addr)
 static uint8_t mbm_get_dest_addr(mb_base_t *inst)
 {
     mbm_object_t *mbm_obj = MB_GET_OBJ_CTX(inst, mbm_object_t, base);
-    return mbm_obj->master_dst_addr;
+    uint8_t dest_addr = 0;
+    CRITICAL_SECTION(inst->lock) {
+        dest_addr = mbm_obj->master_dst_addr;
+    }
+    return dest_addr;
 }
 
 void mbm_error_cb_respond_timeout(mb_base_t *inst, uint8_t dest_addr, const uint8_t *pdu_data, uint16_t pdu_length)
@@ -517,7 +521,6 @@ mb_err_enum_t mbm_poll(mb_base_t *inst)
 {
     mbm_object_t *mbm_obj = MB_GET_OBJ_CTX(inst, mbm_object_t, base);;
 
-    uint16_t length;
     mb_exception_t exception;
     mb_err_enum_t status = MB_ENOERR;
     mb_event_t event;
@@ -599,25 +602,16 @@ mb_err_enum_t mbm_poll(mb_base_t *inst)
                 if (MB_OBJ(inst->transp_obj)->frm_is_bcast(inst->transp_obj)
                         && ((mbm_obj->cur_mode == MB_RTU) || (mbm_obj->cur_mode == MB_ASCII))) {
                     mbm_obj->rcv_frame = mbm_obj->snd_frame;
+                    mbm_obj->pdu_rcv_len = mbm_obj->pdu_snd_len;
+                    mbm_obj->rcv_addr = mbm_obj->master_dst_addr;
                 }
                 MB_RETURN_ON_FALSE(mbm_obj->rcv_frame, MB_EILLSTATE, TAG,
                                    MB_OBJ_FMT", receive buffer initialization fail.", MB_OBJ_PARENT(inst));
                 ESP_LOGD(TAG, MB_OBJ_FMT":EV_EXECUTE", MB_OBJ_PARENT(inst));
                 mbm_obj->func_code = mbm_obj->rcv_frame[MB_PDU_FUNC_OFF];
                 exception = MB_EX_ILLEGAL_FUNCTION;
-                /* If master request is broadcast,
-                 * the master needs to execute function for all slaves.
-                 */
-                if (MB_OBJ(inst->transp_obj)->frm_is_bcast(inst->transp_obj)) {
-                    length = mbm_obj->pdu_snd_len;
-                    for (int j = 1; j <= MB_MASTER_TOTAL_SLAVE_NUM; j++) {
-                        mbm_set_dest_addr(inst, j);
-                        exception = mbm_check_invoke_handler(inst, mbm_obj->func_code, mbm_obj->rcv_frame, &length);
-                    }
-                } else {
-                    ESP_LOGD(TAG, MB_OBJ_FMT": function (0x%x), invoke handler.", MB_OBJ_PARENT(inst), (int)mbm_obj->func_code);
-                    exception = mbm_check_invoke_handler(inst, mbm_obj->func_code, mbm_obj->rcv_frame, &mbm_obj->pdu_rcv_len);
-                }
+                ESP_LOGD(TAG, MB_OBJ_FMT": function (0x%x), addr=%u, invoke handler.", MB_OBJ_PARENT(inst), (int)mbm_obj->func_code, (unsigned)mbm_obj->rcv_addr);
+                exception = mbm_check_invoke_handler(inst, mbm_obj->func_code, mbm_obj->rcv_frame, &mbm_obj->pdu_rcv_len);
                 /* If master has exception, will send error process event. Otherwise the master is idle.*/
                 if (exception != MB_EX_NONE) {
                     mb_port_event_set_err_type(MB_OBJ(inst->port_obj), EV_ERROR_EXECUTE_FUNCTION);

test_apps/broadcast_test/CMakeLists.txt

@@ -0,0 +1,15 @@
+# This is the project CMakeLists.txt file for the test subproject
+cmake_minimum_required(VERSION 3.22)
+
+include($ENV{IDF_PATH}/tools/cmake/project.cmake)
+
+set(EXTRA_COMPONENT_DIRS "../test_common")
+
+# The workaround for the test_utils under ESP-IDF v6.0
+if("${IDF_VERSION_MAJOR}.${IDF_VERSION_MINOR}" VERSION_GREATER "5.5")
+    list(APPEND EXTRA_COMPONENT_DIRS "$ENV{IDF_PATH}/tools/test_apps/components")
+else()
+    list(APPEND EXTRA_COMPONENT_DIRS "$ENV{IDF_PATH}/tools/unit-test-app/components")
+endif()
+
+project(test_broadcast)

test_apps/broadcast_test/README.md

@@ -0,0 +1,4 @@
+| Supported Targets | ESP32 | ESP32-C2 | ESP32-C3 | ESP32-C6 | ESP32-H2 | ESP32-S2 | ESP32-S3 |
+| ----------------- | ----- | -------- | -------- | -------- | -------- | -------- | -------- |
+
+This test app is used to test modbus interface feature.

test_apps/broadcast_test/main/CMakeLists.txt

@@ -0,0 +1,18 @@
+set(PROJECT "test_bcast")
+
+set(srcs "test_app_main.c"
+            "${PROJECT}_serial.c"
+)
+
+idf_component_register(SRCS ${srcs}
+                        PRIV_REQUIRES test_utils test_common unity
+                        )
+
+# The workaround for WHOLE_ARCHIVE which is absent in v4.4
+set_property(TARGET ${COMPONENT_LIB} APPEND PROPERTY INTERFACE_LINK_LIBRARIES "-u mb_test_include_bcast_serial")
+
+# Workaround to avoid static analysis false positives for some components.
+if(CONFIG_FMB_COMPILER_STATIC_ANALYZER_ENABLE AND CMAKE_C_COMPILER_ID STREQUAL "GNU")
+    target_compile_options(${COMPONENT_LIB} PRIVATE "-fanalyzer")
+    message(STATUS "Static analyzer build for ${PROJECT_NAME}.")
+endif()