From de19b81c94e8cdfb5647fb7dd66f8223a59662dd Mon Sep 17 00:00:00 2001 From: aleks Date: Thu, 18 Sep 2025 10:37:47 +0200 Subject: [PATCH] fix master FSM error handling issue --- .../mb_serial_master/main/serial_master.c | 15 +------- examples/tcp/mb_tcp_master/main/tcp_master.c | 2 + modbus/mb_objects/common/mb_types.h | 10 ++--- modbus/mb_objects/mb_master.c | 6 +-- modbus/mb_ports/common/port_common.h | 2 +- modbus/mb_ports/common/port_event.c | 38 ++++++++++++++----- 6 files changed, 40 insertions(+), 33 deletions(-) diff --git a/examples/serial/mb_serial_master/main/serial_master.c b/examples/serial/mb_serial_master/main/serial_master.c index df67a65..47c1b76 100644 --- 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 index 90869eb..f3adb9a 100644 --- 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 index c4d57d1..686903d 100644 --- 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 index fae60ae..0659e7f 100644 --- 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 index 0066005..a6b55f8 100644 --- 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 index 9af44ca..05a85ab 100644 --- a/modbus/mb_ports/common/port_event.c +++ b/modbus/mb_ports/common/port_event.c @@ -161,24 +161,25 @@ 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)) { @@ -186,12 +187,29 @@ mb_err_enum_t mb_port_event_wait_req_finish(mb_port_base_t *inst) 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 {