From 6d72bf12fe8195f65c969503e607b6c55499f9fc 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 --- modbus/mb_objects/common/mb_types.h | 10 ++--- modbus/mb_objects/mb_master.c | 7 ++-- modbus/mb_ports/common/port_common.h | 2 +- modbus/mb_ports/common/port_event.c | 38 ++++++++++++++----- modbus/mb_ports/serial/port_serial.c | 6 +-- modbus/mb_ports/tcp/port_tcp_master.c | 10 ++--- .../test_common/mb_utest_lib/port_adapter.c | 2 - 7 files changed, 44 insertions(+), 31 deletions(-) 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..f6f1564 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; @@ -571,6 +571,7 @@ mb_err_enum_t mbm_poll(mb_base_t *inst) &mbm_obj->rcv_frame, &mbm_obj->pdu_rcv_len); MB_RETURN_ON_FALSE(mbm_obj->snd_frame, MB_EILLSTATE, TAG, "Send buffer initialization fail."); if (event.trans_id == mbm_obj->curr_trans_id) { + mb_port_timer_disable(MB_OBJ(inst->port_obj)); // Check if the frame is for us. If not ,send an error process event. if ((status == MB_ENOERR) && ((mbm_obj->rcv_addr == mbm_obj->master_dst_addr) || (mbm_obj->rcv_addr == MB_TCP_PSEUDO_ADDRESS))) { @@ -594,10 +595,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 { diff --git a/modbus/mb_ports/serial/port_serial.c b/modbus/mb_ports/serial/port_serial.c index c6f3c7b..921b353 100644 --- a/modbus/mb_ports/serial/port_serial.c +++ b/modbus/mb_ports/serial/port_serial.c @@ -155,7 +155,7 @@ static void mb_port_ser_task(void *p_args) // data received during configured timeout and UART TOUT feature is triggered if (event.timeout_flag) { // If bus is busy or fragmented data is received, then flush buffer - if (mb_port_ser_bus_sema_is_busy(&port_obj->base)) { + if (mb_port_ser_bus_sema_is_busy(&port_obj->base) && port_obj->base.descr.is_master) { mb_port_ser_rx_flush(&port_obj->base); break; } @@ -298,8 +298,6 @@ bool mb_port_ser_recv_data(mb_port_base_t *inst, uint8_t **ser_frame, uint16_t * if (status && counter && *ser_frame && atomic_load(&(port_obj->enabled))) { // Read frame data from the ringbuffer of receiver counter = uart_read_bytes(port_obj->ser_opts.port, *ser_frame, counter, MB_SERIAL_RX_TOUT_TICKS); - // Stop timer because the new data is received - mb_port_timer_disable(inst); // Store the timestamp of received frame port_obj->recv_time_stamp = esp_timer_get_time(); ESP_LOGD(TAG, "%s, received data: %d bytes.", inst->descr.parent_name, (int)counter); @@ -331,12 +329,12 @@ bool mb_port_ser_send_data(mb_port_base_t *inst, uint8_t *p_ser_frame, uint16_t count = uart_write_bytes(port_obj->ser_opts.port, p_ser_frame, ser_length); // Waits while UART sending the packet esp_err_t status = uart_wait_tx_done(port_obj->ser_opts.port, MB_SERIAL_TX_TOUT_TICKS); - ESP_LOGD(TAG, "%s, tx buffer sent: (%d) bytes.", inst->descr.parent_name, (int)count); MB_RETURN_ON_FALSE((status == ESP_OK), false, TAG, "%s, mb serial sent buffer failure.", inst->descr.parent_name); MB_PRT_BUF(inst->descr.parent_name, ":PORT_SEND", p_ser_frame, ser_length, ESP_LOG_DEBUG); port_obj->send_time_stamp = esp_timer_get_time(); + res = true; } else { ESP_LOGE(TAG, "%s, send fail state:%d, %p, %u. ", inst->descr.parent_name, (int)port_obj->tx_state_en, p_ser_frame, (unsigned)ser_length); } diff --git a/modbus/mb_ports/tcp/port_tcp_master.c b/modbus/mb_ports/tcp/port_tcp_master.c index b169ca0..097565b 100644 --- a/modbus/mb_ports/tcp/port_tcp_master.c +++ b/modbus/mb_ports/tcp/port_tcp_master.c @@ -234,7 +234,6 @@ bool mbm_port_tcp_recv_data(mb_port_base_t *inst, uint8_t **frame, uint16_t *len ESP_LOGD(TAG, "%p, "MB_NODE_FMT(", get packet TID: 0x%04" PRIx16 ":0x%04" PRIx16 ", %p."), port_obj->drv_obj, info_ptr->index, info_ptr->sock_id, info_ptr->addr_info.ip_addr_str, (unsigned)tid_counter, (unsigned)info_ptr->tid_counter, *frame); - uint64_t time = 0; time = port_get_timestamp() - info_ptr->send_time; ESP_LOGD(TAG, "%p, "MB_NODE_FMT(", processing time[us] = %ju."), port_obj->drv_obj, info_ptr->index, @@ -297,7 +296,8 @@ bool mbm_port_timer_expired(void *inst) BaseType_t task_unblocked; mb_event_info_t mb_event; esp_err_t err = ESP_FAIL; - + + ESP_EARLY_LOGD(TAG, "Timer timeout event: %p", inst); mb_port_timer_disable(inst); // If timer mode is respond timeout, the master event then turns EV_MASTER_EXECUTE status. if (mb_port_get_cur_timer_mode(inst) == MB_TMODE_RESPOND_TIMEOUT) { @@ -331,13 +331,11 @@ static uint64_t mbm_port_tcp_sync_event(void *inst, mb_sync_event_t sync_event) { switch(sync_event) { case MB_SYNC_EVENT_RECV_OK: - mb_port_timer_disable(inst); mb_port_event_set_err_type(inst, EV_ERROR_INIT); mb_port_event_post(inst, EVENT(EV_FRAME_RECEIVED)); break; case MB_SYNC_EVENT_RECV_FAIL: - mb_port_timer_disable(inst); mb_port_event_set_err_type(inst, EV_ERROR_RECEIVE_DATA); mb_port_event_post(inst, EVENT(EV_ERROR_PROCESS)); break; @@ -679,8 +677,10 @@ MB_EVENT_HANDLER(mbm_on_timeout) // Socket read/write timeout is triggered mb_event_info_t *event_info = (mb_event_info_t *)data; ESP_LOGD(TAG, "%s %s: fd: %d", (char *)base, __func__, (int)event_info->opt_fd); - // Todo: this event can be used to check network state (kkep empty for now) + // Todo: this event can be used to check network state (keep empty for now) mb_drv_check_suspend_shutdown(ctx); + // Intentionally allow IDLE task to trigger if other tasks do not perform it properly. + vTaskDelay(1); } #endif diff --git a/test_apps/test_common/mb_utest_lib/port_adapter.c b/test_apps/test_common/mb_utest_lib/port_adapter.c index e87ddfd..07ceb62 100644 --- a/test_apps/test_common/mb_utest_lib/port_adapter.c +++ b/test_apps/test_common/mb_utest_lib/port_adapter.c @@ -576,9 +576,7 @@ bool mb_port_adapter_recv_data(mb_port_base_t *inst, uint8_t **frame_ptr, uint16 CRITICAL_SECTION_LOCK(inst->lock); rx_len = queue_pop(port_obj->rx_queue, &port_obj->rx_buffer[0], CONFIG_FMB_BUFFER_SIZE, NULL); if (rx_len) { - mb_port_timer_disable(inst); ESP_LOGD(TAG, "%s, received data: %d bytes.", inst->descr.parent_name, rx_len); - // Stop timer because the new data is received // Store the timestamp of received frame port_obj->recv_time_stamp = esp_timer_get_time(); *frame_ptr = &port_obj->rx_buffer[0];