fix master FSM error handling issue

alisitsyn · alisitsyn · commit de19b81c94e8 · 2025-09-18T11:35:53.000+02:00
diff --git a/examples/serial/mb_serial_master/main/serial_master.c b/examples/serial/mb_serial_master/main/serial_master.c
@@ -272,7 +272,9 @@ static void *master_get_param_data(const mb_parameter_descriptor_t *param_descri
         if (descr->param_opts.opt3) {                                                             \
             for EACH_ITEM(inst, descr->param_size / sizeof(*item_ptr)) {                          \
                 if (*item_ptr != (typeof(*(inst)))descr->param_opts.opt3) {                       \
+                    mbc_master_lock(handle);                                                      \
                     *item_ptr = (typeof(*(inst)))descr->param_opts.opt3;                          \
+                    mbc_master_unlock(handle);                                                    \
                     ESP_LOGD(TAG, "Characteristic #%d (%s), initialize to 0x%" PRIx16 ".",        \
                                 (int)descr->cid,                                                  \
                                 (char *)descr->param_key,                                         \
@@ -567,24 +569,11 @@ static esp_err_t master_init(void)
     return err;
 }
 
-#include "esp_heap_trace.h"
-
-#define NUM_RECORDS 100
-static heap_trace_record_t trace_record[NUM_RECORDS]; 
-
 void app_main(void)
 {
     // Initialization of device peripheral and objects
     ESP_ERROR_CHECK(master_init());
     vTaskDelay(10);
 
-    ESP_ERROR_CHECK( heap_trace_init_standalone(trace_record, NUM_RECORDS) );
-
-    ESP_ERROR_CHECK( heap_trace_start(HEAP_TRACE_LEAKS) );
-
     master_operation_func(NULL);
-
-    ESP_ERROR_CHECK( heap_trace_stop() );
-    heap_trace_dump();
-
 }
diff --git a/examples/tcp/mb_tcp_master/main/tcp_master.c b/examples/tcp/mb_tcp_master/main/tcp_master.c
@@ -411,7 +411,9 @@ static void *master_get_param_data(const mb_parameter_descriptor_t *param_descri
         if (descr->param_opts.opt3) {                                                             \
             for EACH_ITEM(inst, descr->param_size / sizeof(*item_ptr)) {                          \
                 if (*item_ptr != (typeof(*(inst)))descr->param_opts.opt3) {                       \
+                    mbc_master_lock(handle);                                                      \
                     *item_ptr = (typeof(*(inst)))descr->param_opts.opt3;                          \
+                    mbc_master_lock(handle);                                                      \
                     ESP_LOGD(TAG, "Characteristic #%d (%s), initialize to 0x%" PRIx16 ".",        \
                                 (int)descr->cid,                                                  \
                                 (char *)descr->param_key,                                         \
diff --git a/modbus/mb_objects/common/mb_types.h b/modbus/mb_objects/common/mb_types.h
@@ -105,11 +105,11 @@ typedef mb_exception_t (*mb_fn_handler_fp)(void *, uint8_t *frame_ptr, uint16_t
  * \brief Error event type
  */
 typedef enum mb_err_event_enum {
-    EV_ERROR_INIT,              /*!< No error, initial state. */
-    EV_ERROR_RESPOND_TIMEOUT,   /*!< Slave respond timeout. */
-    EV_ERROR_RECEIVE_DATA,      /*!< Receive frame data error. */
-    EV_ERROR_EXECUTE_FUNCTION,  /*!< Execute function error. */
-    EV_ERROR_OK                 /*!< No error, processing completed. */
+    EV_ERROR_INIT,                      /*!< No error, initial state. */
+    EV_ERROR_RESPOND_TIMEOUT = 0x01,    /*!< Slave respond timeout. */
+    EV_ERROR_RECEIVE_DATA = 0x02,       /*!< Receive frame data error. */
+    EV_ERROR_EXECUTE_FUNCTION = 0x04,   /*!< Execute function error. */
+    EV_ERROR_OK = 0x08                  /*!< No error, processing completed. */
 } mb_err_event_t;
 
 typedef struct mb_event_s {
diff --git a/modbus/mb_objects/mb_master.c b/modbus/mb_objects/mb_master.c
@@ -98,7 +98,7 @@ mb_err_enum_t mbm_get_handler_count(mb_base_t *inst, uint16_t *count)
     return MB_ENOERR;
 }
 
-static mb_err_enum_t mbm_check_invoke_handler(mb_base_t *inst, uint8_t func_code, uint8_t *buf, uint16_t *len)
+static mb_exception_t mbm_check_invoke_handler(mb_base_t *inst, uint8_t func_code, uint8_t *buf, uint16_t *len)
 {
     mbm_object_t *mbm_obj = MB_GET_OBJ_CTX(inst, mbm_object_t, base);
     mb_exception_t exception = MB_EX_ILLEGAL_FUNCTION;
@@ -594,10 +594,8 @@ mb_err_enum_t mbm_poll(mb_base_t *inst)
                     }
                 } else {
                     // Ignore the `EV_FRAME_RECEIVED` event because the respond timeout occurred
-                    // and this is likely respond to previous transaction
+                    // and this is likely respond to previous transaction.
                     ESP_LOGE(TAG, MB_OBJ_FMT", drop data received outside of transaction.", MB_OBJ_PARENT(inst));
-                    mb_port_event_set_err_type(MB_OBJ(inst->port_obj), EV_ERROR_RESPOND_TIMEOUT);
-                    (void)mb_port_event_post(MB_OBJ(inst->port_obj), EVENT(EV_ERROR_PROCESS));
                 }
                 break;
 
diff --git a/modbus/mb_ports/common/port_common.h b/modbus/mb_ports/common/port_common.h
@@ -165,7 +165,7 @@ bool mb_port_event_post(mb_port_base_t *inst, mb_event_t event);
 bool mb_port_event_get(mb_port_base_t *inst, mb_event_t *event);
 bool mb_port_event_res_take(mb_port_base_t *inst, uint32_t timeout);
 void mb_port_event_res_release(mb_port_base_t *inst);
-void mb_port_event_set_resp_flag(mb_port_base_t *inst, mb_event_enum_t event_mask);
+void mb_port_event_set_resp_flag(mb_port_base_t *inst, mb_err_event_t event_mask);
 void mb_port_event_set_err_type(mb_port_base_t *inst, mb_err_event_t event);
 mb_err_event_t mb_port_event_get_err_type(mb_port_base_t *inst);
 void mb_port_event_delete(mb_port_base_t *inst);
diff --git a/modbus/mb_ports/common/port_event.c b/modbus/mb_ports/common/port_event.c
@@ -161,37 +161,55 @@ void mb_port_event_res_release(mb_port_base_t *inst)
     }
 }
 
-void mb_port_event_set_resp_flag(mb_port_base_t *inst, mb_event_enum_t event_mask)
+void mb_port_event_set_resp_flag(mb_port_base_t *inst, mb_err_event_t event_mask)
 {
     MB_RETURN_ON_FALSE((inst), ;, TAG, "incorrect object handle.");
-    (void)xEventGroupSetBits(inst->event_obj->event_group_hdl, event_mask);
+    (void)xEventGroupSetBits(inst->event_obj->event_group_hdl, (EventBits_t)event_mask);
 }
 
 mb_err_enum_t mb_port_event_wait_req_finish(mb_port_base_t *inst)
 {
     MB_RETURN_ON_FALSE((inst), MB_EINVAL, TAG, 
                             "incorrect object handle.");
-    mb_err_enum_t err_status = MB_ENOERR;
-    mb_event_enum_t rcv_event;
-    EventBits_t bits = xEventGroupWaitBits(inst->event_obj->event_group_hdl,// The event group being tested.
-                                                MB_EVENT_REQ_MASK,          // The bits within the event group to wait for.
-                                                pdTRUE,                     // Masked bits should be cleared before returning.
-                                                pdFALSE,                    // Don't wait for both bits, either bit will do.
-                                                MB_EVENT_QUEUE_TIMEOUT_MAX);// Wait forever for either bit to be set.
-    rcv_event = (mb_event_enum_t)(bits);
+    mb_err_enum_t err_status = MB_ETIMEDOUT;
+    mb_err_event_t rcv_event;
+    EventBits_t bits = EV_ERROR_INIT;
+    bits = xEventGroupWaitBits(inst->event_obj->event_group_hdl,                        // The event group being tested.
+                                                MB_EVENT_REQ_MASK,                      // The bits within the event group to wait for.
+                                                pdTRUE,                                 // Masked bits should be cleared before returning.
+                                                pdFALSE,                                // Don't wait for both bits, either bit will do.
+                                                MB_EVENT_QUEUE_TIMEOUT_MAX);            // Wait forever for either bit to be set.
+    rcv_event = (mb_err_event_t)(bits);
     if (rcv_event) {
         ESP_LOGD(TAG, "%s, %s: returned event = 0x%x", inst->descr.parent_name, __func__, (int)rcv_event);
         if (!(rcv_event & MB_EVENT_REQ_MASK)) {
             // if we wait for certain event bits but get from poll subset
             ESP_LOGE(TAG, "%s, %s: incorrect event set = 0x%x", inst->descr.parent_name, __func__, (int)rcv_event);
         }
         if (MB_PORT_CHECK_EVENT(rcv_event, EV_ERROR_OK)) {
+            // Just to check if abnormal state is detected (multiple errors are active). Should not happen in normal FSM handling.
+            if (MB_PORT_CHECK_EVENT(rcv_event, (EV_ERROR_RECEIVE_DATA | EV_ERROR_RESPOND_TIMEOUT | EV_ERROR_EXECUTE_FUNCTION))) {
+                ESP_LOGD(TAG, "%s, %s: multiple errors detected? = 0x%x, clear.", inst->descr.parent_name, __func__, (int)rcv_event);
+                MB_PORT_CLEAR_EVENT(rcv_event, (EV_ERROR_RECEIVE_DATA | EV_ERROR_RESPOND_TIMEOUT | EV_ERROR_EXECUTE_FUNCTION));
+            }
             err_status = MB_ENOERR;
         } else if (MB_PORT_CHECK_EVENT(rcv_event, EV_ERROR_RESPOND_TIMEOUT)) {
+            if (MB_PORT_CHECK_EVENT(rcv_event, (EV_ERROR_RECEIVE_DATA | EV_ERROR_OK | EV_ERROR_EXECUTE_FUNCTION))) {
+                ESP_LOGD(TAG, "%s, %s: multiple errors detected? = 0x%x, clear.", inst->descr.parent_name, __func__, (int)rcv_event);
+                MB_PORT_CLEAR_EVENT(rcv_event, (EV_ERROR_RECEIVE_DATA | EV_ERROR_OK | EV_ERROR_EXECUTE_FUNCTION));
+            }
             err_status = MB_ETIMEDOUT;
         } else if (MB_PORT_CHECK_EVENT(rcv_event, EV_ERROR_RECEIVE_DATA)) {
+            if (MB_PORT_CHECK_EVENT(rcv_event, (EV_ERROR_RESPOND_TIMEOUT | EV_ERROR_OK | EV_ERROR_EXECUTE_FUNCTION))) {
+                ESP_LOGD(TAG, "%s, %s: multiple errors detected? = 0x%x, clear.", inst->descr.parent_name, __func__, (int)rcv_event);
+                MB_PORT_CLEAR_EVENT(rcv_event, (EV_ERROR_RESPOND_TIMEOUT | EV_ERROR_OK | EV_ERROR_EXECUTE_FUNCTION));
+            }
             err_status = MB_ERECVDATA;
         } else if (MB_PORT_CHECK_EVENT(rcv_event, EV_ERROR_EXECUTE_FUNCTION)) {
+            if (MB_PORT_CHECK_EVENT(rcv_event, (EV_ERROR_RECEIVE_DATA | EV_ERROR_OK | EV_ERROR_RESPOND_TIMEOUT))) {
+                ESP_LOGD(TAG, "%s, %s: multiple errors detected? = 0x%x, clear.", inst->descr.parent_name, __func__, (int)rcv_event);
+                MB_PORT_CLEAR_EVENT(rcv_event, (EV_ERROR_RECEIVE_DATA | EV_ERROR_OK | EV_ERROR_RESPOND_TIMEOUT));
+            }
             err_status = MB_EILLFUNC;
         }
     } else {

Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ mb_err_enum_t mbm_get_handler_count(mb_base_t inst, uint16_t count)`
`98`	`98`	`return MB_ENOERR;`
`99`	`99`	`}`
`100`	`100`
`101`		`-static mb_err_enum_t mbm_check_invoke_handler(mb_base_t inst, uint8_t func_code, uint8_t buf, uint16_t *len)`
	`101`	`+static mb_exception_t mbm_check_invoke_handler(mb_base_t inst, uint8_t func_code, uint8_t buf, uint16_t *len)`
`102`	`102`	`{`
`103`	`103`	`mbm_object_t *mbm_obj = MB_GET_OBJ_CTX(inst, mbm_object_t, base);`
`104`	`104`	`mb_exception_t exception = MB_EX_ILLEGAL_FUNCTION;`
`@@ -594,10 +594,8 @@ mb_err_enum_t mbm_poll(mb_base_t *inst)`
`594`	`594`	`}`
`595`	`595`	`} else {`
`596`	`596`	// Ignore the `EV_FRAME_RECEIVED` event because the respond timeout occurred
`597`		`- // and this is likely respond to previous transaction`
	`597`	`+ // and this is likely respond to previous transaction.`
`598`	`598`	`ESP_LOGE(TAG, MB_OBJ_FMT", drop data received outside of transaction.", MB_OBJ_PARENT(inst));`
`599`		`- mb_port_event_set_err_type(MB_OBJ(inst->port_obj), EV_ERROR_RESPOND_TIMEOUT);`
`600`		`- (void)mb_port_event_post(MB_OBJ(inst->port_obj), EVENT(EV_ERROR_PROCESS));`
`601`	`599`	`}`
`602`	`600`	`break;`
`603`	`601`