From d0f724147e6df76742747a1b160ee871f725e49b Mon Sep 17 00:00:00 2001 From: Andrey Gomanchuk Date: Tue, 4 Aug 2026 09:09:29 +0300 Subject: [PATCH 1/5] fix(tcp): reduce driver latency and handle fragmented frames --- modbus/mb_ports/tcp/port_tcp_driver.c | 105 ++++++++--------- modbus/mb_ports/tcp/port_tcp_driver.h | 4 + modbus/mb_ports/tcp/port_tcp_master.c | 16 +-- modbus/mb_ports/tcp/port_tcp_slave.c | 2 +- modbus/mb_ports/tcp/port_tcp_utils.c | 160 +++++++++++--------------- modbus/mb_ports/tcp/port_tcp_utils.h | 1 + 6 files changed, 129 insertions(+), 159 deletions(-) diff --git a/modbus/mb_ports/tcp/port_tcp_driver.c b/modbus/mb_ports/tcp/port_tcp_driver.c index 280de258..ac4158f1 100644 --- a/modbus/mb_ports/tcp/port_tcp_driver.c +++ b/modbus/mb_ports/tcp/port_tcp_driver.c @@ -26,7 +26,6 @@ static const char *TAG = "mb_driver"; -static esp_event_loop_handle_t mb_drv_loop_handle = NULL; static int mb_drv_loop_inst_counter = 0; static char msg_buffer[100]; // The buffer for event debugging (used for all instances) @@ -67,17 +66,19 @@ static esp_err_t init_event_fd(void *ctx) } } drv_obj->event_fd = eventfd(0, 0); - MB_RETURN_ON_FALSE((drv_obj->event_fd > 0), ESP_ERR_INVALID_STATE, TAG, "eventfd init error."); - return (drv_obj->event_fd > 0) ? ESP_OK : ESP_ERR_INVALID_STATE; + MB_RETURN_ON_FALSE((drv_obj->event_fd >= 0), ESP_ERR_INVALID_STATE, TAG, "eventfd init error."); + return (drv_obj->event_fd >= 0) ? ESP_OK : ESP_ERR_INVALID_STATE; } static esp_err_t close_event_fd(void *ctx) { port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); - if (mb_drv_loop_inst_counter) { - close(drv_obj->event_fd); - } else { + if (drv_obj->event_fd >= 0) { ESP_LOGD(TAG, "close eventfd (%d).", (int)drv_obj->event_fd); + close(drv_obj->event_fd); + drv_obj->event_fd = UNDEF_FD; + } + if (!mb_drv_loop_inst_counter) { return esp_vfs_eventfd_unregister(); } return ESP_OK; @@ -87,24 +88,26 @@ int32_t write_event(void *ctx, mb_event_info_t *event) { MB_RETURN_ON_FALSE((event && ctx), -1, TAG, "wrong arguments."); port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); - esp_err_t err = esp_event_post_to(mb_drv_loop_handle, + esp_err_t err = esp_event_post_to(drv_obj->event_loop_hdl, MB_EVENT_BASE(ctx), event->event_id, event, sizeof(mb_event_info_t), MB_EVENT_TOUT); if ((err != ESP_OK)) { ESP_LOGE(TAG, "%p, event loop send fail, err = %d.", ctx, (int)err); return -1; } - // send eventfd to just trigger select - int32_t ret = write(drv_obj->event_fd, (char *)&event->val, sizeof(mb_event_info_t)); - return (ret == sizeof(mb_event_info_t)) ? event->event_id : -1; + atomic_fetch_add(&drv_obj->pending_events, 1); + // eventfd is only a wake-up counter. The event payload is owned by esp_event. + const uint64_t wake_count = 1; + int32_t ret = write(drv_obj->event_fd, &wake_count, sizeof(wake_count)); + return (ret == sizeof(wake_count)) ? event->event_id : -1; } -static int32_t read_event(void *ctx, mb_event_info_t *event) +static uint64_t read_event(void *ctx) { port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); - MB_RETURN_ON_FALSE(event, ESP_ERR_INVALID_STATE, TAG, "cannot get event."); - int ret = read(drv_obj->event_fd, (char *)&event->val, sizeof(mb_event_info_t)); - return (ret == sizeof(mb_event_info_t)) ? event->event_id : -1; + uint64_t event_count = 0; + int ret = read(drv_obj->event_fd, &event_count, sizeof(event_count)); + return (ret == sizeof(event_count)) ? event_count : 0; } static esp_err_t mb_drv_event_loop_init(void *ctx) @@ -116,12 +119,9 @@ static esp_err_t mb_drv_event_loop_init(void *ctx) .queue_size = MB_EVENT_QUEUE_SZ, .task_name = NULL }; - if (!mb_drv_loop_handle && !mb_drv_loop_inst_counter) { - err = esp_event_loop_create(&loop_args, &mb_drv_loop_handle); - MB_RETURN_ON_FALSE(((err == ESP_OK) && mb_drv_loop_handle), ESP_ERR_INVALID_STATE, - TAG, "create event loop failed, err=%d.", (int)err); - } - drv_obj->event_loop_hdl = mb_drv_loop_handle; + err = esp_event_loop_create(&loop_args, &drv_obj->event_loop_hdl); + MB_RETURN_ON_FALSE(((err == ESP_OK) && drv_obj->event_loop_hdl), ESP_ERR_INVALID_STATE, + TAG, "create event loop failed, err=%d.", (int)err); if (asprintf(&drv_obj->loop_name, "loop:%p", ctx) == -1) { abort(); } @@ -133,21 +133,19 @@ static esp_err_t mb_drv_event_loop_deinit(void *ctx) port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); esp_err_t err = ESP_OK; // delete event loop - MB_RETURN_ON_FALSE((mb_drv_loop_handle), ESP_ERR_INVALID_STATE, + MB_RETURN_ON_FALSE((drv_obj->event_loop_hdl), ESP_ERR_INVALID_STATE, TAG, "delete event loop failed."); + ESP_LOGD(TAG, "delete loop inst: %s.", drv_obj->loop_name); + free(drv_obj->loop_name); + drv_obj->loop_name = NULL; if (mb_drv_loop_inst_counter) { - ESP_LOGD(TAG, "delete loop inst: %s.", drv_obj->loop_name); - free(drv_obj->loop_name); - drv_obj->loop_name = NULL; mb_drv_loop_inst_counter--; } - if (!mb_drv_loop_inst_counter) { - err = esp_event_loop_delete(mb_drv_loop_handle); - ESP_LOGD(TAG, "delete event loop: %p.", mb_drv_loop_handle); - mb_drv_loop_handle = NULL; - MB_RETURN_ON_FALSE((err == ESP_OK), ESP_ERR_INVALID_STATE, - TAG, "delete event loop failed, error=%d.", (int)err); - } + err = esp_event_loop_delete(drv_obj->event_loop_hdl); + ESP_LOGD(TAG, "delete event loop: %p.", drv_obj->event_loop_hdl); + drv_obj->event_loop_hdl = NULL; + MB_RETURN_ON_FALSE((err == ESP_OK), ESP_ERR_INVALID_STATE, + TAG, "delete event loop failed, error=%d.", (int)err); return err; } @@ -161,7 +159,7 @@ esp_err_t mb_drv_register_handler(void *ctx, mb_driver_event_num_t event_num, mb MB_RETURN_ON_FALSE((drv_obj->event_handler[event_num] == NULL), ESP_ERR_INVALID_ARG, TAG, "%p, event handler %p, for event %x, is not empty.", drv_obj, drv_obj->event_handler[event_num], (int)event); - ret = esp_event_handler_instance_register_with(mb_drv_loop_handle, MB_EVENT_BASE(ctx), event, + ret = esp_event_handler_instance_register_with(drv_obj->event_loop_hdl, MB_EVENT_BASE(ctx), event, fp, ctx, &drv_obj->event_handler[event_num]); ESP_LOGD(TAG, "%p, registered event handler %p, event 0x%x", drv_obj, drv_obj->event_handler[event_num], (int)event); MB_RETURN_ON_FALSE((ret == ESP_OK), ESP_ERR_INVALID_STATE, @@ -180,7 +178,7 @@ esp_err_t mb_drv_unregister_handler(void *ctx, mb_driver_event_num_t event_num) MB_RETURN_ON_FALSE((drv_obj->event_handler[event_num]), ESP_ERR_INVALID_ARG, TAG, "%p, event handler %p, for event %x, is incorrect.", drv_obj, drv_obj->event_handler[event_num], (int)event); - ret = esp_event_handler_instance_unregister_with(mb_drv_loop_handle, + ret = esp_event_handler_instance_unregister_with(drv_obj->event_loop_hdl, MB_EVENT_BASE(ctx), (int32_t)event, drv_obj->event_handler[event_num]); drv_obj->event_handler[event_num] = NULL; MB_RETURN_ON_FALSE((ret == ESP_OK), ESP_ERR_INVALID_STATE, @@ -461,9 +459,8 @@ mb_node_info_t *mb_drv_get_next_node_from_set(void *ctx, int *fd_ptr, fd_set *fd node_ptr = drv_obj->mb_nodes[fd]; if (node_ptr && (node_ptr->sock_id > 0) && (MB_GET_NODE_STATE(node_ptr) >= MB_SOCK_STATE_CONNECTED) - && (FD_ISSET(node_ptr->index, fdset) || (FD_ISSET(node_ptr->sock_id, fdset)))) { + && FD_ISSET(node_ptr->sock_id, fdset)) { *fd_ptr = fd; - //FD_CLR(node_ptr->sock_id, fdset); return node_ptr; } } @@ -620,15 +617,18 @@ void mb_drv_tcp_task(void *ctx) // Is the fd event triggered, process the event if (drv_obj->event_fd && FD_ISSET(drv_obj->event_fd, &readset)) { FD_CLR(drv_obj->event_fd, &readset); - mb_event_info_t mb_event = {0}; - int32_t event_id = read_event(ctx, &mb_event); - ESP_LOGD(TAG, "%p, fd event get: 0x%02x:%d, %s", - ctx, (int)event_id, (int)mb_event.opt_fd, driver_event_to_name_r(event_id)); + uint64_t wake_count = read_event(ctx); + uint32_t event_count = atomic_exchange(&drv_obj->pending_events, 0); + ESP_LOGD(TAG, "%p, fd wake count: %" PRIu64 ", event count: %" PRIu32, + ctx, wake_count, event_count); mb_drv_check_suspend_shutdown(ctx); - // Drive the event loop - esp_err_t err = esp_event_loop_run(mb_drv_loop_handle, pdMS_TO_TICKS(MB_TCP_EVENT_LOOP_TICK_MS)); - if (err != ESP_OK) { - ESP_LOGE(TAG, "%p, event loop run, returns fail: %x", ctx, (int)err); + // A zero timeout dispatches one queued event without waiting for another one. + for (uint32_t event = 0; event < event_count; event++) { + esp_err_t err = esp_event_loop_run(drv_obj->event_loop_hdl, 0); + if (err != ESP_OK) { + ESP_LOGE(TAG, "%p, event loop run, returns fail: %x", ctx, (int)err); + break; + } } } if (drv_obj->listen_sock_fd && FD_ISSET(drv_obj->listen_sock_fd, &readset)) { @@ -659,7 +659,7 @@ void mb_drv_tcp_task(void *ctx) mb_node_info_t *node_ptr = NULL; while (((node_ptr = mb_drv_get_next_node_from_set(ctx, &curr_fd, &readset)) && (curr_fd < MB_MAX_FDS))) { - if (FD_ISSET(node_ptr->sock_id, &drv_obj->conn_set)) { + if (FD_ISSET(node_ptr->sock_id, &readset)) { // The data is ready in the socket, read frame and queue FD_CLR(node_ptr->sock_id, &readset); int ret = port_read_packet(node_ptr); @@ -671,7 +671,7 @@ void mb_drv_tcp_task(void *ctx) mb_drv_unlock(ctx); DRIVER_SEND_EVENT(ctx, MB_EVENT_RECV_DATA, node_ptr->index); } else if (ret == ERR_TIMEOUT) { - ESP_LOGD(TAG, "%p, "MB_NODE_FMT(", frame read timeout or closed connection."), ctx, (int)node_ptr->fd, + ESP_LOGD(TAG, "%p, "MB_NODE_FMT(", frame is not complete yet."), ctx, (int)node_ptr->fd, (int)node_ptr->sock_id, node_ptr->addr_info.ip_addr_str); } else if (ret == ERR_BUF) { // After retries a response with incorrect TID received, process failure. @@ -732,8 +732,6 @@ esp_err_t mb_drv_register(port_driver_t **ctx) MB_GOTO_ON_FALSE((pctx->status_flags_hdl), ESP_ERR_INVALID_STATE, error, TAG, "%p, mb event group error.", pctx); - mb_drv_loop_inst_counter++; - // Create task for packet processing BaseType_t state = xTaskCreatePinnedToCore(mb_drv_tcp_task, "mb_drv_tcp_task", @@ -744,6 +742,7 @@ esp_err_t mb_drv_register(port_driver_t **ctx) MB_PORT_TASK_AFFINITY); MB_GOTO_ON_FALSE((state == pdTRUE), ESP_ERR_INVALID_STATE, error, TAG, "%p, event task creation error.", pctx); + mb_drv_loop_inst_counter++; (void)mb_drv_stop_task(pctx); *ctx = pctx; @@ -757,15 +756,17 @@ esp_err_t mb_drv_register(port_driver_t **ctx) if (pctx->mb_tcp_task_handle) { vTaskDelete(pctx->mb_tcp_task_handle); } - if (mb_drv_loop_handle) { - (void)esp_event_loop_delete(mb_drv_loop_handle); - mb_drv_loop_handle = NULL; + if (pctx->event_loop_hdl) { + (void)esp_event_loop_delete(pctx->event_loop_hdl); + pctx->event_loop_hdl = NULL; free(pctx->loop_name); pctx->loop_name = NULL; } - if (pctx->event_fd) { + if (pctx->event_fd >= 0) { close(pctx->event_fd); - (void)esp_vfs_eventfd_unregister(); + if (!mb_drv_loop_inst_counter) { + (void)esp_vfs_eventfd_unregister(); + } } if (pctx->close_done_sema) { vSemaphoreDelete(pctx->close_done_sema); diff --git a/modbus/mb_ports/tcp/port_tcp_driver.h b/modbus/mb_ports/tcp/port_tcp_driver.h index 42c7eae8..2fab2a3c 100644 --- a/modbus/mb_ports/tcp/port_tcp_driver.h +++ b/modbus/mb_ports/tcp/port_tcp_driver.h @@ -207,6 +207,9 @@ typedef struct mb_node_info_s { int recv_err; /*!< socket receive error */ QueueHandle_t rx_queue; /*!< receive response queue */ QueueHandle_t tx_queue; /*!< send request queue */ + uint8_t rx_buffer[MB_TCP_BUFF_MAX_SIZE]; /*!< partially received TCP frame */ + uint16_t rx_length; /*!< number of bytes accumulated in rx_buffer */ + uint16_t rx_expected_length; /*!< complete frame length, zero until MBAP header is received */ int64_t send_time; /*!< send request time stamp */ int64_t recv_time; /*!< receive response time stamp */ uint16_t tid_counter; /*!< transaction identifier (TID) for slave */ @@ -269,6 +272,7 @@ typedef struct _port_driver { EventGroupHandle_t status_flags_hdl; /*!< status bits to control nodes states */ TaskHandle_t mb_tcp_task_handle; /*!< TCP/UDP handling task handle */ esp_event_loop_handle_t event_loop_hdl; /*!< event loop handle */ + _Atomic(uint32_t) pending_events; /*!< number of events waiting for dispatch */ esp_event_handler_instance_t event_handler[MB_EVENT_COUNT]; /*!< event handler instance */ char *loop_name; /*!< name for event loop used as base */ mb_driver_event_cb_t event_cbs; diff --git a/modbus/mb_ports/tcp/port_tcp_master.c b/modbus/mb_ports/tcp/port_tcp_master.c index 818b03c1..043ec33c 100644 --- a/modbus/mb_ports/tcp/port_tcp_master.c +++ b/modbus/mb_ports/tcp/port_tcp_master.c @@ -295,25 +295,12 @@ void mbm_port_tcp_set_conn_cb(mb_port_base_t *inst, void *conn_fp, void *arg) // Timer handler to check timeout of socket response bool mbm_port_timer_expired(void *inst) { - mbm_tcp_port_t *port_obj = __containerof(inst, mbm_tcp_port_t, base); bool need_poll = false; - 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) { - // It is now to check solution. - mb_event.event_id = MB_EVENT_TIMEOUT; - mb_event.opt_fd = port_obj->drv_obj->curr_node_index; - err = esp_event_isr_post_to(port_obj->drv_obj->event_loop_hdl, MB_EVENT_BASE(port_obj->drv_obj), - (int32_t)MB_EVENT_TIMEOUT, (void *)&mb_event, sizeof(mb_event_info_t *), &task_unblocked); - if (err != ESP_OK) { - ESP_EARLY_LOGE(TAG, "Timeout event send error: %d", err); - } - need_poll = task_unblocked; mb_port_event_set_err_type(inst, EV_ERROR_RESPOND_TIMEOUT); need_poll = mb_port_event_post(inst, EVENT(EV_ERROR_PROCESS)); } @@ -468,6 +455,7 @@ MB_EVENT_HANDLER(mbm_on_connect) node_ptr->addr_info.ip_addr_str); MB_SET_NODE_STATE(node_ptr, MB_SOCK_STATE_CONNECTED); (void)port_keep_alive_enable(node_ptr->sock_id, CONFIG_FMB_TCP_KEEP_ALIVE_TOUT_SEC); + (void)port_tcp_set_no_delay(node_ptr->sock_id); ESP_LOGD(TAG, "Opened/connected: %u, %u.", (unsigned)drv_obj->mb_node_open_count, (unsigned)drv_obj->node_conn_count); if (drv_obj->mb_node_open_count == drv_obj->node_conn_count) { @@ -714,8 +702,6 @@ MB_EVENT_HANDLER(mbm_on_timeout) 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 (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/modbus/mb_ports/tcp/port_tcp_slave.c b/modbus/mb_ports/tcp/port_tcp_slave.c index a263ada8..80145561 100644 --- a/modbus/mb_ports/tcp/port_tcp_slave.c +++ b/modbus/mb_ports/tcp/port_tcp_slave.c @@ -389,6 +389,7 @@ MB_EVENT_HANDLER(mbs_on_connect) return; } (void)port_keep_alive_enable(pnode->sock_id, CONFIG_FMB_TCP_KEEP_ALIVE_TOUT_SEC); + (void)port_tcp_set_no_delay(pnode->sock_id); mb_drv_lock(ctx); MB_SET_NODE_STATE(pnode, MB_SOCK_STATE_CONNECTED); FD_SET(pnode->sock_id, &drv_obj->conn_set); @@ -702,7 +703,6 @@ MB_EVENT_HANDLER(mbs_on_timeout) } else { curr_fd++; } - vTaskDelay(1); } #endif diff --git a/modbus/mb_ports/tcp/port_tcp_utils.c b/modbus/mb_ports/tcp/port_tcp_utils.c index 3a1d2280..1697f1ed 100644 --- a/modbus/mb_ports/tcp/port_tcp_utils.c +++ b/modbus/mb_ports/tcp/port_tcp_utils.c @@ -158,122 +158,90 @@ int port_dequeue_packet(QueueHandle_t queue, frame_entry_t *frame_info_ptr) return ERR_BUF; } -static int port_get_buf(mb_node_info_t *info_ptr, uint8_t *pdst_buf, uint16_t len, uint16_t read_tick_ms) +static void port_reset_rx(mb_node_info_t *info_ptr) { - int ret = 0; - uint8_t *buf = pdst_buf; - uint16_t bytes_left = len; - struct timeval time_val; - - MB_RETURN_ON_FALSE((info_ptr && (info_ptr->sock_id > UNDEF_FD)), -1, TAG, "Try to read incorrect socket = #%d.", info_ptr->sock_id); - - // Set receive timeout for socket <= slave respond time - time_val.tv_sec = read_tick_ms / 1000; - time_val.tv_usec = (read_tick_ms % 1000) * 1000; - setsockopt(info_ptr->sock_id, SOL_SOCKET, SO_RCVTIMEO, &time_val, sizeof(time_val)); - - // blocking read of data from socket - ret = recv(info_ptr->sock_id, buf, bytes_left, 0); - if (ret == 0) { - return ERR_CONN; // FIN received, peer closed - } - if (ret < 0) { - ESP_LOGD(TAG, "socket(#%d)(%s) recv return, ret=%d, errno=%d.", - info_ptr->sock_id, info_ptr->addr_info.ip_addr_str, ret, (int)errno); - if (errno == EINPROGRESS || errno == EAGAIN || errno == EWOULDBLOCK) { - // Read timeout occurred, check the timeout and return - return 0; - } - if ((errno == ENOTCONN) || (errno == ECONNRESET)) { - ESP_LOGD(TAG, "socket(#%d)(%s) connection closed, ret=%d, errno=%d.", - info_ptr->sock_id, info_ptr->addr_info.ip_addr_str, ret, (int)errno); - // Socket connection closed - return ERR_CONN; - } - // Other error occurred during receiving - ESP_LOGD(TAG, "Socket(#%d)(%s) receive error, ret = %d, errno = %d(%s)", - info_ptr->sock_id, info_ptr->addr_info.ip_addr_str, ret, (int)errno, strerror(errno)); - ret = -1; - } - return ret; + info_ptr->rx_length = 0; + info_ptr->rx_expected_length = 0; } int port_read_packet(mb_node_info_t *info_ptr) { - uint16_t temp = 0; - int ret = 0; - uint8_t ptemp_buf[MB_TCP_BUFF_MAX_SIZE] = {0}; + MB_RETURN_ON_FALSE((info_ptr && (info_ptr->sock_id > 0)), -1, TAG, + "try to read incorrect socket = #%d", info_ptr ? info_ptr->sock_id : UNDEF_FD); - // Receive data from connected client - if (info_ptr) { - MB_RETURN_ON_FALSE((info_ptr->sock_id > 0), -1, TAG, "try to read incorrect socket = #%d", info_ptr->sock_id); - // Read packet header - ret = port_get_buf(info_ptr, ptemp_buf, MB_TCP_UID, MB_READ_TICK); + while (true) { + uint16_t target_length = info_ptr->rx_expected_length; + if (!target_length) { + target_length = MB_TCP_UID; + } + + size_t bytes_left = target_length - info_ptr->rx_length; + ssize_t ret = recv(info_ptr->sock_id, &info_ptr->rx_buffer[info_ptr->rx_length], + bytes_left, MSG_DONTWAIT); if (ret == 0) { - ESP_LOGD(TAG, "node #%d, Socket (#%d)(%s), socket connection is closed or timeout, err=%d, ", - info_ptr->fd, info_ptr->sock_id, info_ptr->addr_info.ip_addr_str, ret); + port_reset_rx(info_ptr); + info_ptr->recv_err = ERR_CONN; return ERR_CONN; } - if (ret < 0) { - info_ptr->recv_err = ret; - return ret; - } - - if (ret != MB_TCP_UID) { - ESP_LOGD(TAG, "node #%d, Socket (#%d)(%s), fail to read modbus header, err=%d", - info_ptr->fd, info_ptr->sock_id, info_ptr->addr_info.ip_addr_str, ret); - info_ptr->recv_err = ERR_VAL; - return ERR_VAL; + if ((errno == EINPROGRESS) || (errno == EAGAIN) || (errno == EWOULDBLOCK)) { + info_ptr->recv_err = ERR_TIMEOUT; + return ERR_TIMEOUT; + } + if ((errno == ENOTCONN) || (errno == ECONNRESET)) { + port_reset_rx(info_ptr); + info_ptr->recv_err = ERR_CONN; + return ERR_CONN; + } + ESP_LOGD(TAG, "Socket(#%d)(%s) receive error, ret=%d, errno=%d(%s)", + info_ptr->sock_id, info_ptr->addr_info.ip_addr_str, + (int)ret, (int)errno, strerror(errno)); + port_reset_rx(info_ptr); + info_ptr->recv_err = ERR_IF; + return ERR_IF; } - temp = MB_TCP_MBAP_GET_FIELD(ptemp_buf, MB_TCP_PID); - if (temp != 0) { - info_ptr->recv_err = ERR_BUF; - return ERR_BUF; + info_ptr->rx_length += (uint16_t)ret; + if (info_ptr->rx_length < target_length) { + continue; } - // If we have received the MBAP header we can analyze it and calculate - // the number of bytes left to complete the current response. - // Second chunk is stored from ptemp_buf[MB_TCP_UID], so temp must be - // <= (MB_TCP_BUFF_MAX_SIZE - MB_TCP_UID) to fit in ptemp_buf[]. - temp = MB_TCP_MBAP_GET_FIELD(ptemp_buf, MB_TCP_LEN); - const uint16_t mbap_payload_max = (uint16_t)(MB_TCP_BUFF_MAX_SIZE - MB_TCP_UID); - if (temp > mbap_payload_max) { - ESP_LOGD(TAG, "Incorrect packet length: %u (max %u)", (unsigned)temp, (unsigned)mbap_payload_max); - ESP_LOG_BUFFER_HEX_LEVEL(TAG, ptemp_buf, MB_TCP_FUNC, ESP_LOG_DEBUG); - info_ptr->recv_err = ERR_BUF; - temp = mbap_payload_max; // read all remaining data from buffer - } - // Sequential frame read with minimal timeout to reduce delays - ret = port_get_buf(info_ptr, &ptemp_buf[MB_TCP_UID], temp, MB_READ_TICK); - if (ret < 0) { - info_ptr->recv_err = ret; - return ret; - } + if (!info_ptr->rx_expected_length) { + uint16_t protocol_id = MB_TCP_MBAP_GET_FIELD(info_ptr->rx_buffer, MB_TCP_PID); + uint16_t payload_length = MB_TCP_MBAP_GET_FIELD(info_ptr->rx_buffer, MB_TCP_LEN); + const uint16_t payload_max = (uint16_t)(MB_TCP_BUFF_MAX_SIZE - MB_TCP_UID); - if ((ret < temp) || (ret < MB_PDU_SIZE_MIN)) { - info_ptr->recv_err = ERR_VAL; - return ERR_VAL; + const uint16_t payload_min = (uint16_t)(MB_TCP_FUNC - MB_TCP_UID + MB_PDU_SIZE_MIN); + if ((protocol_id != 0) || (payload_length < payload_min) || (payload_length > payload_max)) { + ESP_LOGD(TAG, "Invalid MBAP header: pid=%u, length=%u.", + (unsigned)protocol_id, (unsigned)payload_length); + ESP_LOG_BUFFER_HEX_LEVEL(TAG, info_ptr->rx_buffer, MB_TCP_UID, ESP_LOG_DEBUG); + port_reset_rx(info_ptr); + info_ptr->recv_err = ERR_BUF; + return ERR_BUF; + } + info_ptr->rx_expected_length = MB_TCP_UID + payload_length; + continue; } - if (ptemp_buf[MB_TCP_UID] > MB_ADDRESS_MAX) { + uint16_t frame_length = info_ptr->rx_expected_length; + if (info_ptr->rx_buffer[MB_TCP_UID] > MB_ADDRESS_MAX) { + port_reset_rx(info_ptr); info_ptr->recv_err = ERR_BUF; return ERR_BUF; } - ret = port_enqueue_packet(info_ptr->rx_queue, ptemp_buf, temp + MB_TCP_UID); - if (ret < 0) { - info_ptr->recv_err = ret; - return ret; + int enqueue_result = port_enqueue_packet(info_ptr->rx_queue, info_ptr->rx_buffer, frame_length); + port_reset_rx(info_ptr); + if (enqueue_result < 0) { + info_ptr->recv_err = enqueue_result; + return enqueue_result; } info_ptr->recv_counter++; - info_ptr->recv_err = ERR_OK; - return ret + MB_TCP_FUNC; + return frame_length; } - return -1; } err_t port_set_blocking(mb_node_info_t *info_ptr, bool is_blocking) @@ -305,6 +273,16 @@ int mb_set_linger(int sock, int tout) return res; } +int port_tcp_set_no_delay(int sock) +{ + int enabled = 1; + int ret = setsockopt(sock, IPPROTO_TCP, TCP_NODELAY, &enabled, sizeof(enabled)); + if (ret != 0) { + ESP_LOGE(TAG, "Sock %d, set TCP_NODELAY fail, errno=%d.", sock, (int)errno); + } + return ret; +} + int port_keep_alive_enable(int sock, int timeout_sec) { if ((timeout_sec > 7200) || (timeout_sec < 1)) { @@ -507,7 +485,7 @@ int port_write_poll(mb_node_info_t *info_ptr, const uint8_t *frame, uint16_t fra info_ptr->index, info_ptr->sock_id, info_ptr->addr_info.ip_addr_str, ret, (int)errno); return ret; } - ret = send(info_ptr->sock_id, frame, frame_len, TCP_NODELAY); + ret = send(info_ptr->sock_id, frame, frame_len, 0); if (ret < 0) { ESP_LOGE(TAG, MB_NODE_FMT(", send data error: %d, errno %d"), info_ptr->index, info_ptr->sock_id, info_ptr->addr_info.ip_addr_str, ret, (int)errno); diff --git a/modbus/mb_ports/tcp/port_tcp_utils.h b/modbus/mb_ports/tcp/port_tcp_utils.h index 3ab24688..dec6c96c 100644 --- a/modbus/mb_ports/tcp/port_tcp_utils.h +++ b/modbus/mb_ports/tcp/port_tcp_utils.h @@ -112,6 +112,7 @@ int port_enqueue_packet(QueueHandle_t queue, uint8_t *buf, uint16_t len); int port_dequeue_packet(QueueHandle_t queue, frame_entry_t *frame_info); int port_read_packet(mb_node_info_t *info_ptr); err_t port_set_blocking(mb_node_info_t *info_ptr, bool is_blocking); +int port_tcp_set_no_delay(int sock); int port_keep_alive_enable(int sock, int timeout_sec); err_t port_check_alive(mb_node_info_t *info_ptr, uint32_t timeout_ms); err_t port_connect(void *ctx, mb_node_info_t *info_ptr); From 904323002ff32507ab0206fbd327742be37ff2c3 Mon Sep 17 00:00:00 2001 From: Andrey Gomanchuk Date: Tue, 4 Aug 2026 09:09:29 +0300 Subject: [PATCH 2/5] refactor(tcp): replace esp event loop with direct dispatch --- CMakeLists.txt | 3 +- modbus/mb_ports/tcp/port_tcp_driver.c | 132 +++++++++----------------- modbus/mb_ports/tcp/port_tcp_driver.h | 28 ++---- modbus/mb_ports/tcp/port_tcp_master.c | 2 +- modbus/mb_ports/tcp/port_tcp_master.h | 1 - modbus/mb_ports/tcp/port_tcp_slave.c | 2 +- 6 files changed, 57 insertions(+), 111 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 10f4811c..f27ef7d3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -63,13 +63,12 @@ if("${IDF_VERSION_MAJOR}.${IDF_VERSION_MINOR}" VERSION_GREATER "5.2") set(requires esp_driver_uart) set(priv_requires esp_timer # mb timer implementation - esp_event # mb tcp event loops esp_netif # mb tcp workaround for ipv6 vfs # mb tcp uses event_fd ) else() set(requires driver) -set(priv_requires esp_timer esp_event esp_netif vfs) +set(priv_requires esp_timer esp_netif vfs) endif() diff --git a/modbus/mb_ports/tcp/port_tcp_driver.c b/modbus/mb_ports/tcp/port_tcp_driver.c index ac4158f1..6b75567c 100644 --- a/modbus/mb_ports/tcp/port_tcp_driver.c +++ b/modbus/mb_ports/tcp/port_tcp_driver.c @@ -4,7 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ #include -#include #include #include #include "errno.h" @@ -26,7 +25,7 @@ static const char *TAG = "mb_driver"; -static int mb_drv_loop_inst_counter = 0; +static int mb_drv_inst_counter = 0; static char msg_buffer[100]; // The buffer for event debugging (used for all instances) static const event_msg_t event_msg_table[] = { @@ -58,7 +57,7 @@ const char *driver_event_to_name_r(mb_driver_event_t event) static esp_err_t init_event_fd(void *ctx) { port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); - if (!mb_drv_loop_inst_counter) { + if (!mb_drv_inst_counter) { esp_vfs_eventfd_config_t config = MB_EVENTFD_CONFIG(); esp_err_t err = esp_vfs_eventfd_register(&config); if ((err != ESP_OK) && (err != ESP_ERR_INVALID_STATE)) { @@ -78,7 +77,7 @@ static esp_err_t close_event_fd(void *ctx) close(drv_obj->event_fd); drv_obj->event_fd = UNDEF_FD; } - if (!mb_drv_loop_inst_counter) { + if (!mb_drv_inst_counter) { return esp_vfs_eventfd_unregister(); } return ESP_OK; @@ -88,15 +87,10 @@ int32_t write_event(void *ctx, mb_event_info_t *event) { MB_RETURN_ON_FALSE((event && ctx), -1, TAG, "wrong arguments."); port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); - esp_err_t err = esp_event_post_to(drv_obj->event_loop_hdl, - MB_EVENT_BASE(ctx), event->event_id, event, - sizeof(mb_event_info_t), MB_EVENT_TOUT); - if ((err != ESP_OK)) { - ESP_LOGE(TAG, "%p, event loop send fail, err = %d.", ctx, (int)err); + if (xQueueSend(drv_obj->event_queue, event, MB_EVENT_TOUT) != pdTRUE) { + ESP_LOGE(TAG, "%p, event queue is full.", ctx); return -1; } - atomic_fetch_add(&drv_obj->pending_events, 1); - // eventfd is only a wake-up counter. The event payload is owned by esp_event. const uint64_t wake_count = 1; int32_t ret = write(drv_obj->event_fd, &wake_count, sizeof(wake_count)); return (ret == sizeof(wake_count)) ? event->event_id : -1; @@ -110,80 +104,50 @@ static uint64_t read_event(void *ctx) return (ret == sizeof(event_count)) ? event_count : 0; } -static esp_err_t mb_drv_event_loop_init(void *ctx) +static void mb_drv_dispatch_events(void *ctx, uint64_t event_count) { port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); - esp_err_t err = ESP_OK; - /* Create Event loop without task (will be created separately)*/ - esp_event_loop_args_t loop_args = { - .queue_size = MB_EVENT_QUEUE_SZ, - .task_name = NULL - }; - err = esp_event_loop_create(&loop_args, &drv_obj->event_loop_hdl); - MB_RETURN_ON_FALSE(((err == ESP_OK) && drv_obj->event_loop_hdl), ESP_ERR_INVALID_STATE, - TAG, "create event loop failed, err=%d.", (int)err); - if (asprintf(&drv_obj->loop_name, "loop:%p", ctx) == -1) { - abort(); + mb_event_info_t event_info; + while (event_count-- && (xQueueReceive(drv_obj->event_queue, &event_info, 0) == pdTRUE)) { + mb_driver_event_num_t event_num = MB_EVENT_READY_NUM; + while ((event_num < MB_EVENT_COUNT) && (MB_EVENT_FROM_NUM(event_num) != event_info.event_id)) { + event_num++; + } + if ((event_num < MB_EVENT_COUNT) && drv_obj->event_handler[event_num]) { + drv_obj->event_handler[event_num](ctx, TAG, event_info.event_id, &event_info); + } else { + ESP_LOGE(TAG, "%p, no handler for event 0x%x.", ctx, (unsigned)event_info.event_id); + } } - return err; -} - -static esp_err_t mb_drv_event_loop_deinit(void *ctx) -{ - port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); - esp_err_t err = ESP_OK; - // delete event loop - MB_RETURN_ON_FALSE((drv_obj->event_loop_hdl), ESP_ERR_INVALID_STATE, - TAG, "delete event loop failed."); - ESP_LOGD(TAG, "delete loop inst: %s.", drv_obj->loop_name); - free(drv_obj->loop_name); - drv_obj->loop_name = NULL; - if (mb_drv_loop_inst_counter) { - mb_drv_loop_inst_counter--; - } - err = esp_event_loop_delete(drv_obj->event_loop_hdl); - ESP_LOGD(TAG, "delete event loop: %p.", drv_obj->event_loop_hdl); - drv_obj->event_loop_hdl = NULL; - MB_RETURN_ON_FALSE((err == ESP_OK), ESP_ERR_INVALID_STATE, - TAG, "delete event loop failed, error=%d.", (int)err); - return err; } esp_err_t mb_drv_register_handler(void *ctx, mb_driver_event_num_t event_num, mb_event_handler_fp fp) { port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); - esp_err_t ret = ESP_ERR_INVALID_STATE; + MB_RETURN_ON_FALSE(((event_num >= MB_EVENT_READY_NUM) && (event_num < MB_EVENT_COUNT) && fp), + ESP_ERR_INVALID_ARG, + TAG, "%p, event number %d is out of range.", drv_obj, (int)event_num); mb_driver_event_t event = MB_EVENT_FROM_NUM(event_num); - ESP_LOGD(TAG, "%p, event #%d, 0x%x, register.", drv_obj, (int)event_num, (int)event); MB_RETURN_ON_FALSE((drv_obj->event_handler[event_num] == NULL), ESP_ERR_INVALID_ARG, TAG, "%p, event handler %p, for event %x, is not empty.", drv_obj, drv_obj->event_handler[event_num], (int)event); - - ret = esp_event_handler_instance_register_with(drv_obj->event_loop_hdl, MB_EVENT_BASE(ctx), event, - fp, ctx, &drv_obj->event_handler[event_num]); + drv_obj->event_handler[event_num] = fp; ESP_LOGD(TAG, "%p, registered event handler %p, event 0x%x", drv_obj, drv_obj->event_handler[event_num], (int)event); - MB_RETURN_ON_FALSE((ret == ESP_OK), ESP_ERR_INVALID_STATE, - TAG, "%p, event handler %p, registration error.", drv_obj, drv_obj->event_handler[event_num]); - return ESP_OK; } esp_err_t mb_drv_unregister_handler(void *ctx, mb_driver_event_num_t event_num) { port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); - esp_err_t ret = ESP_ERR_INVALID_STATE; + MB_RETURN_ON_FALSE(((event_num >= MB_EVENT_READY_NUM) && (event_num < MB_EVENT_COUNT)), + ESP_ERR_INVALID_ARG, + TAG, "%p, event number %d is out of range.", drv_obj, (int)event_num); mb_driver_event_t event = MB_EVENT_FROM_NUM(event_num); - ESP_LOGD(TAG, "%p, event handler %p, event 0x%x, unregister.", drv_obj, drv_obj->event_handler[event_num], (int)event); MB_RETURN_ON_FALSE((drv_obj->event_handler[event_num]), ESP_ERR_INVALID_ARG, TAG, "%p, event handler %p, for event %x, is incorrect.", drv_obj, drv_obj->event_handler[event_num], (int)event); - ret = esp_event_handler_instance_unregister_with(drv_obj->event_loop_hdl, - MB_EVENT_BASE(ctx), (int32_t)event, drv_obj->event_handler[event_num]); drv_obj->event_handler[event_num] = NULL; - MB_RETURN_ON_FALSE((ret == ESP_OK), ESP_ERR_INVALID_STATE, - TAG, "%p, event handler %p, instance unregister with, error = %d", drv_obj, drv_obj->event_handler[event_num], (int)ret); - return ESP_OK; } @@ -615,23 +579,14 @@ void mb_drv_tcp_task(void *ctx) ESP_LOGD(TAG, "%p, socket error, fdset: %" PRIx64, ctx, *(uint64_t *)&errorset); } else { // Is the fd event triggered, process the event - if (drv_obj->event_fd && FD_ISSET(drv_obj->event_fd, &readset)) { + if ((drv_obj->event_fd >= 0) && FD_ISSET(drv_obj->event_fd, &readset)) { FD_CLR(drv_obj->event_fd, &readset); - uint64_t wake_count = read_event(ctx); - uint32_t event_count = atomic_exchange(&drv_obj->pending_events, 0); - ESP_LOGD(TAG, "%p, fd wake count: %" PRIu64 ", event count: %" PRIu32, - ctx, wake_count, event_count); + uint64_t event_count = read_event(ctx); + ESP_LOGD(TAG, "%p, event count: %" PRIu64, ctx, event_count); mb_drv_check_suspend_shutdown(ctx); - // A zero timeout dispatches one queued event without waiting for another one. - for (uint32_t event = 0; event < event_count; event++) { - esp_err_t err = esp_event_loop_run(drv_obj->event_loop_hdl, 0); - if (err != ESP_OK) { - ESP_LOGE(TAG, "%p, event loop run, returns fail: %x", ctx, (int)err); - break; - } - } + mb_drv_dispatch_events(ctx, event_count); } - if (drv_obj->listen_sock_fd && FD_ISSET(drv_obj->listen_sock_fd, &readset)) { + if ((drv_obj->listen_sock_fd >= 0) && FD_ISSET(drv_obj->listen_sock_fd, &readset)) { // If something happened on the listen socket, then it is an incoming connection. FD_CLR(drv_obj->listen_sock_fd, &readset); ESP_LOGD(TAG, "%p, listen_sock is active.", ctx); @@ -724,9 +679,9 @@ esp_err_t mb_drv_register(port_driver_t **ctx) MB_GOTO_ON_FALSE((ret == ESP_OK), ESP_ERR_INVALID_STATE, error, TAG, "%p, vfs eventfd init error.", pctx); - ret = mb_drv_event_loop_init((void *)pctx); - MB_GOTO_ON_FALSE((ret == ESP_OK), ESP_ERR_INVALID_STATE, error, - TAG, "%p, event loop init error.", pctx); + pctx->event_queue = xQueueCreate(MB_EVENT_QUEUE_SZ, sizeof(mb_event_info_t)); + MB_GOTO_ON_FALSE((pctx->event_queue), ESP_ERR_NO_MEM, error, + TAG, "%p, event queue allocation error.", pctx); pctx->status_flags_hdl = xEventGroupCreate(); MB_GOTO_ON_FALSE((pctx->status_flags_hdl), ESP_ERR_INVALID_STATE, error, @@ -742,7 +697,7 @@ esp_err_t mb_drv_register(port_driver_t **ctx) MB_PORT_TASK_AFFINITY); MB_GOTO_ON_FALSE((state == pdTRUE), ESP_ERR_INVALID_STATE, error, TAG, "%p, event task creation error.", pctx); - mb_drv_loop_inst_counter++; + mb_drv_inst_counter++; (void)mb_drv_stop_task(pctx); *ctx = pctx; @@ -756,15 +711,13 @@ esp_err_t mb_drv_register(port_driver_t **ctx) if (pctx->mb_tcp_task_handle) { vTaskDelete(pctx->mb_tcp_task_handle); } - if (pctx->event_loop_hdl) { - (void)esp_event_loop_delete(pctx->event_loop_hdl); - pctx->event_loop_hdl = NULL; - free(pctx->loop_name); - pctx->loop_name = NULL; + if (pctx->event_queue) { + vQueueDelete(pctx->event_queue); + pctx->event_queue = NULL; } if (pctx->event_fd >= 0) { close(pctx->event_fd); - if (!mb_drv_loop_inst_counter) { + if (!mb_drv_inst_counter) { (void)esp_vfs_eventfd_unregister(); } } @@ -794,7 +747,9 @@ esp_err_t mb_drv_unregister(void *ctx) vTaskDelete(drv_obj->mb_tcp_task_handle); } - mb_drv_event_loop_deinit(ctx); + if (mb_drv_inst_counter) { + mb_drv_inst_counter--; + } if (drv_obj->close_done_sema) { vSemaphoreDelete(drv_obj->close_done_sema); drv_obj->close_done_sema = NULL; @@ -805,7 +760,12 @@ esp_err_t mb_drv_unregister(void *ctx) ESP_LOGE(TAG, "could not close the eventfd handle, err = %d. Already closed?", err); } - if (drv_obj->listen_sock_fd) { + if (drv_obj->event_queue) { + vQueueDelete(drv_obj->event_queue); + drv_obj->event_queue = NULL; + } + + if (drv_obj->listen_sock_fd >= 0) { shutdown(drv_obj->listen_sock_fd, SHUT_RDWR); close(drv_obj->listen_sock_fd); drv_obj->listen_sock_fd = UNDEF_FD; diff --git a/modbus/mb_ports/tcp/port_tcp_driver.h b/modbus/mb_ports/tcp/port_tcp_driver.h index 2fab2a3c..ded42112 100644 --- a/modbus/mb_ports/tcp/port_tcp_driver.h +++ b/modbus/mb_ports/tcp/port_tcp_driver.h @@ -5,14 +5,11 @@ */ #pragma once -#include - #include "esp_err.h" #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "freertos/semphr.h" #include "freertos/queue.h" -#include "esp_event.h" // for esp event loop #if __has_include("mdns.h") #include "mdns.h" @@ -35,8 +32,8 @@ extern "C" { #define MB_EVENT_TOUT (300 / portTICK_PERIOD_MS) #define MB_CONN_TICK_TIMEOUT (10 / portTICK_PERIOD_MS) -typedef void (*mb_event_handler_fp)(void *ctx, esp_event_base_t base, int32_t id, void *data); -#define MB_EVENT_HANDLER(handler_name) void (handler_name)(void *ctx, esp_event_base_t base, int32_t id, void *data) +typedef void (*mb_event_handler_fp)(void *ctx, const char *base, int32_t id, void *data); +#define MB_EVENT_HANDLER(handler_name) void (handler_name)(void *ctx, const char *base, int32_t id, void *data) #define MB_TASK_STACK_SZ (CONFIG_FMB_PORT_TASK_STACK_SIZE) #define MB_TASK_PRIO (CONFIG_FMB_PORT_TASK_PRIO) @@ -70,6 +67,7 @@ typedef void (*mb_event_handler_fp)(void *ctx, esp_event_base_t base, int32_t id .close_done_sema = NULL, \ .node_conn_count = 0, \ .event_fd = UNDEF_FD, \ + .event_queue = NULL, \ } #define MB_EVENTFD_CONFIG() (esp_vfs_eventfd_config_t) { \ @@ -89,15 +87,8 @@ typedef struct _port_driver port_driver_t; #define MB_EVENT_TBL_IT(event) {event, #event} -#define MB_EVENT_BASE(context) (__extension__( \ -{ \ - port_driver_t *drv_obj = MB_GET_DRV_PTR(context); \ - (drv_obj->loop_name) ? (esp_event_base_t)(drv_obj->loop_name) : "UNK_BASE"; \ -} \ -)) - #define MB_ADD_FD(fd, max_fd, fdset) do { \ - if (fd) { \ + if ((fd) >= 0) { \ (max_fd = (fd > max_fd) ? fd : max_fd); \ FD_SET(fd, fdset); \ } \ @@ -119,8 +110,7 @@ typedef struct _port_driver port_driver_t; } \ )) -// Post event to event loop and unblocks the select through the eventfd to handle the event loop run, -// So, the eventfd value keeps last event and its fd. +// Queue an event and unblock select through eventfd. #define DRIVER_SEND_EVENT_MACRO(ctx, event, fd, value) (__extension__( \ { \ port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); \ @@ -267,14 +257,12 @@ typedef struct _port_driver { uint16_t curr_node_index; /*!< current processing slave index */ fd_set open_set; /*!< file descriptor set for opened nodes */ fd_set conn_set; /*!< file descriptor set for associated nodes */ - int event_fd; /*!< eventfd descriptor for modbus event tracking */ + int event_fd; /*!< eventfd used to wake select */ + QueueHandle_t event_queue; /*!< queued driver events */ SemaphoreHandle_t close_done_sema; /*!< close and done semaphore */ EventGroupHandle_t status_flags_hdl; /*!< status bits to control nodes states */ TaskHandle_t mb_tcp_task_handle; /*!< TCP/UDP handling task handle */ - esp_event_loop_handle_t event_loop_hdl; /*!< event loop handle */ - _Atomic(uint32_t) pending_events; /*!< number of events waiting for dispatch */ - esp_event_handler_instance_t event_handler[MB_EVENT_COUNT]; /*!< event handler instance */ - char *loop_name; /*!< name for event loop used as base */ + mb_event_handler_fp event_handler[MB_EVENT_COUNT]; /*!< driver event handlers */ mb_driver_event_cb_t event_cbs; //LIST_HEAD(mb_uid_info_, mb_uid_entry_s) node_list; /*!< node address information list */ //uint16_t node_list_count; diff --git a/modbus/mb_ports/tcp/port_tcp_master.c b/modbus/mb_ports/tcp/port_tcp_master.c index 043ec33c..aa4b0396 100644 --- a/modbus/mb_ports/tcp/port_tcp_master.c +++ b/modbus/mb_ports/tcp/port_tcp_master.c @@ -687,7 +687,7 @@ MB_EVENT_HANDLER(mbm_on_close) pnode = mb_drv_get_node(drv_obj, event_info->opt_fd); if (pnode && (MB_GET_NODE_STATE(pnode) >= MB_SOCK_STATE_OPENED)) { ESP_LOGD(TAG, "%p, Close node %d, sock #%d, intentionally.", ctx, (int)event_info->opt_fd, pnode->sock_id); - if ((pnode->sock_id < 0) && FD_ISSET(pnode->sock_id, &drv_obj->open_set)) { + if (FD_ISSET(pnode->index, &drv_obj->open_set)) { mb_drv_close(drv_obj, event_info->opt_fd); } } diff --git a/modbus/mb_ports/tcp/port_tcp_master.h b/modbus/mb_ports/tcp/port_tcp_master.h index 1791a9df..d7572987 100644 --- a/modbus/mb_ports/tcp/port_tcp_master.h +++ b/modbus/mb_ports/tcp/port_tcp_master.h @@ -10,7 +10,6 @@ #include "freertos/task.h" #include "freertos/semphr.h" #include "freertos/queue.h" -#include "esp_event.h" // for esp event loop #include "mb_common.h" #include "mb_frame.h" diff --git a/modbus/mb_ports/tcp/port_tcp_slave.c b/modbus/mb_ports/tcp/port_tcp_slave.c index 80145561..07438069 100644 --- a/modbus/mb_ports/tcp/port_tcp_slave.c +++ b/modbus/mb_ports/tcp/port_tcp_slave.c @@ -667,7 +667,7 @@ MB_EVENT_HANDLER(mbs_on_close) } else if (MB_CHECK_FD_RANGE(event_info->opt_fd)) { pnode = mb_drv_get_node(drv_obj, event_info->opt_fd); if (pnode && (MB_GET_NODE_STATE(pnode) >= MB_SOCK_STATE_OPENED)) { - if ((pnode->sock_id < 0) && FD_ISSET(pnode->sock_id, &drv_obj->open_set)) { + if (FD_ISSET(pnode->index, &drv_obj->open_set)) { mb_drv_lock(drv_obj); (void)transaction_delete_by_node_id(port_obj->transaction, event_info->opt_fd); mb_drv_unlock(drv_obj); From 36b44d64feece3ee00142f58cadae4669474b974 Mon Sep 17 00:00:00 2001 From: Andrey Gomanchuk Date: Tue, 4 Aug 2026 09:09:29 +0300 Subject: [PATCH 3/5] refactor(tcp): remove obsolete event loop remnants --- Kconfig | 11 ------- modbus/mb_ports/tcp/port_tcp_driver.c | 30 +------------------- modbus/mb_ports/tcp/port_tcp_driver.h | 24 ++++------------ modbus/mb_ports/tcp/port_tcp_master.c | 17 ++++++----- modbus/mb_ports/tcp/port_tcp_slave.c | 41 ++++++++++++++++++--------- 5 files changed, 42 insertions(+), 81 deletions(-) diff --git a/Kconfig b/Kconfig index d558c7ad..014ec6a7 100644 --- a/Kconfig +++ b/Kconfig @@ -81,17 +81,6 @@ menu "Modbus configuration" Before each send, the driver verifies the socket is writable via select function. Reduce for faster failure detection; increase on high-latency links. - config FMB_TCP_EVENT_LOOP_TICK_MS - int "Modbus TCP event loop processing budget (ms)" - range 10 300 - default 50 - depends on FMB_COMM_MODE_TCP_EN - help - This option represents the maximum time allocated to event loop run function per driver loop iteration. - The value controls how many queued events (connect, send, error, etc.) are dispatched - before returning to check sockets. Lower values reduce latency for socket - I/O at the cost of processing fewer events per iteration. - config FMB_TCP_UID_ENABLED bool "Modbus TCP enable UID (Unit Identifier) support" default n diff --git a/modbus/mb_ports/tcp/port_tcp_driver.c b/modbus/mb_ports/tcp/port_tcp_driver.c index 6b75567c..276dac3e 100644 --- a/modbus/mb_ports/tcp/port_tcp_driver.c +++ b/modbus/mb_ports/tcp/port_tcp_driver.c @@ -3,7 +3,6 @@ * * SPDX-License-Identifier: Apache-2.0 */ -#include #include #include #include "errno.h" @@ -26,33 +25,6 @@ static const char *TAG = "mb_driver"; static int mb_drv_inst_counter = 0; -static char msg_buffer[100]; // The buffer for event debugging (used for all instances) - -static const event_msg_t event_msg_table[] = { - MB_EVENT_TBL_IT(MB_EVENT_READY), - MB_EVENT_TBL_IT(MB_EVENT_OPEN), - MB_EVENT_TBL_IT(MB_EVENT_RESOLVE), - MB_EVENT_TBL_IT(MB_EVENT_CONNECT), - MB_EVENT_TBL_IT(MB_EVENT_SEND_DATA), - MB_EVENT_TBL_IT(MB_EVENT_RECV_DATA), - MB_EVENT_TBL_IT(MB_EVENT_ERROR), - MB_EVENT_TBL_IT(MB_EVENT_CLOSE), - MB_EVENT_TBL_IT(MB_EVENT_TIMEOUT), -}; - -// The function to print event -const char *driver_event_to_name_r(mb_driver_event_t event) -{ - msg_buffer[0] = 0; - size_t i; - for (i = 0; i < sizeof(event_msg_table) / sizeof(event_msg_table[0]); ++i) { - if (event_msg_table[i].event & event) { - strlcat(msg_buffer, "|", sizeof(msg_buffer)); - strlcat(msg_buffer, event_msg_table[i].msg, sizeof(msg_buffer)); - } - } - return msg_buffer; -} static esp_err_t init_event_fd(void *ctx) { @@ -114,7 +86,7 @@ static void mb_drv_dispatch_events(void *ctx, uint64_t event_count) event_num++; } if ((event_num < MB_EVENT_COUNT) && drv_obj->event_handler[event_num]) { - drv_obj->event_handler[event_num](ctx, TAG, event_info.event_id, &event_info); + drv_obj->event_handler[event_num](ctx, &event_info); } else { ESP_LOGE(TAG, "%p, no handler for event 0x%x.", ctx, (unsigned)event_info.event_id); } diff --git a/modbus/mb_ports/tcp/port_tcp_driver.h b/modbus/mb_ports/tcp/port_tcp_driver.h index ded42112..428c52ef 100644 --- a/modbus/mb_ports/tcp/port_tcp_driver.h +++ b/modbus/mb_ports/tcp/port_tcp_driver.h @@ -30,10 +30,9 @@ extern "C" { #define MB_PORT_DEFAULT (CONFIG_FMB_TCP_PORT_DEFAULT) #define UNDEF_FD (-1) #define MB_EVENT_TOUT (300 / portTICK_PERIOD_MS) -#define MB_CONN_TICK_TIMEOUT (10 / portTICK_PERIOD_MS) -typedef void (*mb_event_handler_fp)(void *ctx, const char *base, int32_t id, void *data); -#define MB_EVENT_HANDLER(handler_name) void (handler_name)(void *ctx, const char *base, int32_t id, void *data) +typedef void (*mb_event_handler_fp)(void *ctx, void *data); +#define MB_EVENT_HANDLER(handler_name) void (handler_name)(void *ctx, void *data) #define MB_TASK_STACK_SZ (CONFIG_FMB_PORT_TASK_STACK_SIZE) #define MB_TASK_PRIO (CONFIG_FMB_PORT_TASK_PRIO) @@ -50,7 +49,6 @@ typedef void (*mb_event_handler_fp)(void *ctx, const char *base, int32_t id, voi #define MB_WAIT_DONE_MS (5000) #define MB_SELECT_WAIT_MS (CONFIG_FMB_TCP_EVENT_WAIT_MS) #define MB_TCP_SEND_TIMEOUT_MS (CONFIG_FMB_TCP_SEND_TIMEOUT_MS) -#define MB_TCP_EVENT_LOOP_TICK_MS (CONFIG_FMB_TCP_EVENT_LOOP_TICK_MS) #define MB_DRIVER_CONFIG_DEFAULT { \ .spin_lock = portMUX_INITIALIZER_UNLOCKED, \ @@ -85,8 +83,6 @@ typedef struct _port_driver port_driver_t; } \ )) -#define MB_EVENT_TBL_IT(event) {event, #event} - #define MB_ADD_FD(fd, max_fd, fdset) do { \ if ((fd) >= 0) { \ (max_fd = (fd > max_fd) ? fd : max_fd); \ @@ -175,17 +171,9 @@ typedef enum _mb_driver_event { } mb_driver_event_t; typedef struct { - mb_driver_event_t event; - const char *msg; -} event_msg_t; - -typedef union { - struct { - int32_t event_id; /*!< an event */ - int16_t opt_fd; /*!< fd option for an event */ - int16_t opt_val; /*!< value option for an event */ - }; - uint64_t val; + int32_t event_id; /*!< an event */ + int16_t opt_fd; /*!< fd option for an event */ + int16_t opt_val; /*!< value option for an event */ } mb_event_info_t; typedef struct mb_node_info_s { @@ -334,8 +322,6 @@ int mb_drv_close(void *ctx, int fd); int32_t write_event(void *ctx, mb_event_info_t *event); -const char *driver_event_to_name_r(mb_driver_event_t event); - void mb_drv_set_cb(void *ctx, void *conn_cb, void *arg); mb_status_flags_t mb_drv_wait_status_flag(void *ctx, mb_status_flags_t mask, TickType_t ticks); diff --git a/modbus/mb_ports/tcp/port_tcp_master.c b/modbus/mb_ports/tcp/port_tcp_master.c index aa4b0396..1bdd6779 100644 --- a/modbus/mb_ports/tcp/port_tcp_master.c +++ b/modbus/mb_ports/tcp/port_tcp_master.c @@ -354,22 +354,21 @@ static uint64_t mbm_port_tcp_sync_event(void *inst, mb_sync_event_t sync_event) MB_EVENT_HANDLER(mbm_on_ready) { - // The driver is registered 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); + ESP_LOGD(TAG, "%s: fd: %d", __func__, (int)event_info->opt_fd); } MB_EVENT_HANDLER(mbm_on_open) { 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); + ESP_LOGD(TAG, "%s: fd: %d", __func__, (int)event_info->opt_fd); } MB_EVENT_HANDLER(mbm_on_resolve) { port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); 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); + ESP_LOGD(TAG, "%s: fd: %d", __func__, (int)event_info->opt_fd); if (MB_CHECK_FD_RANGE(event_info->opt_fd)) { ESP_LOGD(TAG, "%p, Node: %d, resolve.", ctx, (int)event_info->opt_fd); @@ -431,7 +430,7 @@ MB_EVENT_HANDLER(mbm_on_connect) port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); mb_node_info_t *node_ptr = NULL; 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); + ESP_LOGD(TAG, "%s: fd: %d", __func__, (int)event_info->opt_fd); err_t err = ERR_CONN; if (MB_CHECK_FD_RANGE(event_info->opt_fd)) { node_ptr = mb_drv_get_node(drv_obj, event_info->opt_fd); @@ -567,7 +566,7 @@ MB_EVENT_HANDLER(mbm_on_send_data) { port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); 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); + ESP_LOGD(TAG, "%s: fd: %d", __func__, (int)event_info->opt_fd); mb_drv_check_suspend_shutdown(ctx); mb_node_info_t *info_ptr = mb_drv_get_node(drv_obj, event_info->opt_fd); if (info_ptr && !queue_is_empty(info_ptr->tx_queue)) { @@ -628,7 +627,7 @@ MB_EVENT_HANDLER(mbm_on_recv_data) { port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); 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); + ESP_LOGD(TAG, "%s: fd: %d", __func__, (int)event_info->opt_fd); uint8_t buf[MB_TCP_BUFF_MAX_SIZE] = {0}; mb_drv_check_suspend_shutdown(ctx); // Get frame from queue, check for correctness, push back correct frame and generate receive condition. @@ -664,7 +663,7 @@ MB_EVENT_HANDLER(mbm_on_recv_data) MB_EVENT_HANDLER(mbm_on_close) { 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); + ESP_LOGD(TAG, "%s, fd: %d", __func__, (int)event_info->opt_fd); port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); mb_node_info_t *pnode = NULL; // if close all sockets event is received @@ -699,7 +698,7 @@ 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); + ESP_LOGD(TAG, "%s: fd: %d", __func__, (int)event_info->opt_fd); // Todo: this event can be used to check network state (keep empty for now) mb_drv_check_suspend_shutdown(ctx); } diff --git a/modbus/mb_ports/tcp/port_tcp_slave.c b/modbus/mb_ports/tcp/port_tcp_slave.c index 07438069..96bb1f11 100644 --- a/modbus/mb_ports/tcp/port_tcp_slave.c +++ b/modbus/mb_ports/tcp/port_tcp_slave.c @@ -46,6 +46,9 @@ static esp_err_t mbs_port_tcp_register_handlers(void *ctx) ret = mb_drv_register_handler(drv_obj, MB_EVENT_OPEN_NUM, mbs_on_open); MB_RETURN_ON_FALSE((ret == ESP_OK), MB_EINVAL, TAG, "%x, mb tcp port event registration failed.", (int)MB_EVENT_OPEN); + ret = mb_drv_register_handler(drv_obj, MB_EVENT_RESOLVE_NUM, mbs_on_resolve); + MB_RETURN_ON_FALSE((ret == ESP_OK), MB_EINVAL, TAG, + "%x, mb tcp port event registration failed.", (int)MB_EVENT_RESOLVE); ret = mb_drv_register_handler(drv_obj, MB_EVENT_CONNECT_NUM, mbs_on_connect); MB_RETURN_ON_FALSE((ret == ESP_OK), MB_EINVAL, TAG, "%x, mb tcp port event registration failed.", (int)MB_EVENT_CONNECT); @@ -79,9 +82,15 @@ static esp_err_t mbs_port_tcp_unregister_handlers(void *ctx) ret = mb_drv_unregister_handler(drv_obj, MB_EVENT_OPEN_NUM); MB_RETURN_ON_FALSE((ret == ESP_OK), MB_EINVAL, TAG, "%x, mb tcp port event registration failed.", (int)MB_EVENT_OPEN); + ret = mb_drv_unregister_handler(drv_obj, MB_EVENT_RESOLVE_NUM); + MB_RETURN_ON_FALSE((ret == ESP_OK), MB_EINVAL, TAG, + "%x, mb tcp port event registration failed.", (int)MB_EVENT_RESOLVE); ret = mb_drv_unregister_handler(drv_obj, MB_EVENT_CONNECT_NUM); MB_RETURN_ON_FALSE((ret == ESP_OK), MB_EINVAL, TAG, "%x, mb tcp port event registration failed.", (int)MB_EVENT_CONNECT); + ret = mb_drv_unregister_handler(drv_obj, MB_EVENT_ERROR_NUM); + MB_RETURN_ON_FALSE((ret == ESP_OK), MB_EINVAL, TAG, + "%x, mb tcp port event registration failed.", (int)MB_EVENT_ERROR); ret = mb_drv_unregister_handler(drv_obj, MB_EVENT_SEND_DATA_NUM); MB_RETURN_ON_FALSE((ret == ESP_OK), MB_EINVAL, TAG, "%x, mb tcp port event registration failed.", (int)MB_EVENT_SEND_DATA); @@ -334,7 +343,7 @@ MB_EVENT_HANDLER(mbs_on_ready) mb_event_info_t *event_info = (mb_event_info_t *)data; port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); mbs_tcp_port_t *port_obj = __containerof(drv_obj->parent, mbs_tcp_port_t, base); - ESP_LOGD(TAG, "%s %s: fd: %d", (char *)base, __func__, (int)event_info->opt_fd); + ESP_LOGD(TAG, "%s: fd: %d", __func__, (int)event_info->opt_fd); ESP_LOGD(TAG, "addr_table:%p, addr_type:%d, mode:%d, port:%d", port_obj->tcp_opts.ip_addr_table, (int)port_obj->tcp_opts.addr_type, (int)port_obj->tcp_opts.mode, @@ -346,7 +355,7 @@ MB_EVENT_HANDLER(mbs_on_ready) port_obj->tcp_opts.port); if (listen_sock < 0) { mb_drv_check_suspend_shutdown(ctx); - ESP_LOGE(TAG, "%s, sock: %d, bind error", (char *)base, listen_sock); + ESP_LOGE(TAG, "sock: %d, bind error", listen_sock); mb_drv_lock(drv_obj); if (drv_obj->retry_cnt) { drv_obj->retry_cnt--; @@ -357,7 +366,7 @@ MB_EVENT_HANDLER(mbs_on_ready) DRIVER_SEND_EVENT(ctx, MB_EVENT_READY, UNDEF_FD); } else { DRIVER_SEND_EVENT(ctx, MB_EVENT_CLOSE, UNDEF_FD); - ESP_LOGE(TAG, "%s, stop binding.", (char *)base); + ESP_LOGE(TAG, "stop binding."); // mbs_port_tcp_disable(&port_obj->base); } } else { @@ -368,24 +377,30 @@ MB_EVENT_HANDLER(mbs_on_ready) (void)mb_drv_set_status_flag(drv_obj, MB_FLAG_TRANSACTION_READY); mb_drv_unlock(ctx); drv_obj->event_cbs.mb_sync_event_cb(drv_obj->event_cbs.port_arg, MB_SYNC_EVENT_READY); - ESP_LOGI(TAG, "%s %s: fd: %d, bind is done", (char *)base, __func__, (int)event_info->opt_fd); + ESP_LOGI(TAG, "%s: fd: %d, bind is done", __func__, (int)event_info->opt_fd); } } MB_EVENT_HANDLER(mbs_on_open) { 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); + ESP_LOGD(TAG, "%s: fd: %d", __func__, (int)event_info->opt_fd); +} + +MB_EVENT_HANDLER(mbs_on_resolve) +{ + mb_event_info_t *event_info = (mb_event_info_t *)data; + ESP_LOGD(TAG, "%s: fd: %d", __func__, (int)event_info->opt_fd); } MB_EVENT_HANDLER(mbs_on_connect) { mb_event_info_t *event_info = (mb_event_info_t *)data; port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); - ESP_LOGD(TAG, "%s %s: fd: %d", (char *)base, __func__, (int)event_info->opt_fd); + ESP_LOGD(TAG, "%s: fd: %d", __func__, (int)event_info->opt_fd); mb_node_info_t *pnode = mb_drv_get_node(drv_obj, event_info->opt_fd); if (!pnode) { - ESP_LOGD(TAG, "%s %s: fd: %d, is closed.", (char *)base, __func__, (int)event_info->opt_fd); + ESP_LOGD(TAG, "%s: fd: %d, is closed.", __func__, (int)event_info->opt_fd); return; } (void)port_keep_alive_enable(pnode->sock_id, CONFIG_FMB_TCP_KEEP_ALIVE_TOUT_SEC); @@ -404,7 +419,7 @@ MB_EVENT_HANDLER(mbs_on_recv_data) port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); mb_event_info_t *event_info = (mb_event_info_t *)data; mbs_tcp_port_t *port_obj = (mbs_tcp_port_t *)drv_obj->parent; - ESP_LOGD(TAG, "%s %s: fd: %d", (char *)base, __func__, (int)event_info->opt_fd); + ESP_LOGD(TAG, "%s: fd: %d", __func__, (int)event_info->opt_fd); mb_node_info_t *pnode = mb_drv_get_node(drv_obj, event_info->opt_fd); transaction_item_handle_t item = NULL; if (pnode) { @@ -490,7 +505,7 @@ MB_EVENT_HANDLER(mbs_on_send_data) esp_err_t err = ESP_ERR_INVALID_STATE; frame_entry_t frame_entry = {0}; int ret = 0; - ESP_LOGD(TAG, "%s %s: fd: %d", (char *)base, __func__, (int)event_info->opt_fd); + ESP_LOGD(TAG, "%s: fd: %d", __func__, (int)event_info->opt_fd); mb_node_info_t *pnode = mb_drv_get_node(drv_obj, event_info->opt_fd); if (pnode && !queue_is_empty(pnode->tx_queue)) { // Pop the frame entry, keep the buffer @@ -607,10 +622,10 @@ MB_EVENT_HANDLER(mbs_on_error) port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); mb_event_info_t *event_info = (mb_event_info_t *)data; mbs_tcp_port_t *port_obj = __containerof(drv_obj->parent, mbs_tcp_port_t, base); - ESP_LOGD(TAG, "%s %s: fd: %d", (char *)base, __func__, (int)event_info->opt_fd); + ESP_LOGD(TAG, "%s: fd: %d", __func__, (int)event_info->opt_fd); mb_node_info_t *pnode = mb_drv_get_node(drv_obj, event_info->opt_fd); if (!pnode) { - ESP_LOGD(TAG, "%s %s: fd: %d, is closed.", (char *)base, __func__, (int)event_info->opt_fd); + ESP_LOGD(TAG, "%s: fd: %d, is closed.", __func__, (int)event_info->opt_fd); return; } mb_drv_check_suspend_shutdown(ctx); @@ -648,7 +663,7 @@ MB_EVENT_HANDLER(mbs_on_error) MB_EVENT_HANDLER(mbs_on_close) { 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); + ESP_LOGD(TAG, "%s, fd: %d", __func__, (int)event_info->opt_fd); port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); mbs_tcp_port_t *port_obj = __containerof(drv_obj->parent, mbs_tcp_port_t, base); mb_node_info_t *pnode = NULL; @@ -686,7 +701,7 @@ MB_EVENT_HANDLER(mbs_on_timeout) mbs_tcp_port_t *port_obj = __containerof(drv_obj->parent, mbs_tcp_port_t, base); static int curr_fd = 0; mb_node_info_t *pnode = mb_drv_get_node(drv_obj, curr_fd); - ESP_LOGD(TAG, "%s %s: fd: %d, count: %d", (char *)base, __func__, (int)curr_fd, drv_obj->node_conn_count); + ESP_LOGD(TAG, "%s: fd: %d, count: %d", __func__, (int)curr_fd, drv_obj->node_conn_count); mb_drv_check_suspend_shutdown(ctx); int ret = mb_drv_check_node_state(drv_obj, &curr_fd, CONFIG_FMB_TCP_CONNECTION_TOUT_SEC * 1000); if ((ret != ERR_OK) && (ret != ERR_TIMEOUT)) { From c7112373bfc0b4a7e873014fc27505c071caf3dd Mon Sep 17 00:00:00 2001 From: Andrey Gomanchuk Date: Tue, 4 Aug 2026 09:20:59 +0300 Subject: [PATCH 4/5] perf(tcp): avoid redundant internal eventfd wakeups --- modbus/mb_ports/tcp/port_tcp_driver.c | 25 ++++++++++++++++++------- modbus/mb_ports/tcp/port_tcp_driver.h | 1 + 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/modbus/mb_ports/tcp/port_tcp_driver.c b/modbus/mb_ports/tcp/port_tcp_driver.c index 276dac3e..cdf1a065 100644 --- a/modbus/mb_ports/tcp/port_tcp_driver.c +++ b/modbus/mb_ports/tcp/port_tcp_driver.c @@ -59,10 +59,15 @@ int32_t write_event(void *ctx, mb_event_info_t *event) { MB_RETURN_ON_FALSE((event && ctx), -1, TAG, "wrong arguments."); port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); - if (xQueueSend(drv_obj->event_queue, event, MB_EVENT_TOUT) != pdTRUE) { + bool is_driver_task = (xTaskGetCurrentTaskHandle() == drv_obj->mb_tcp_task_handle); + TickType_t timeout = is_driver_task ? 0 : MB_EVENT_TOUT; + if (xQueueSend(drv_obj->event_queue, event, timeout) != pdTRUE) { ESP_LOGE(TAG, "%p, event queue is full.", ctx); return -1; } + if (is_driver_task) { + return event->event_id; + } const uint64_t wake_count = 1; int32_t ret = write(drv_obj->event_fd, &wake_count, sizeof(wake_count)); return (ret == sizeof(wake_count)) ? event->event_id : -1; @@ -76,11 +81,14 @@ static uint64_t read_event(void *ctx) return (ret == sizeof(event_count)) ? event_count : 0; } -static void mb_drv_dispatch_events(void *ctx, uint64_t event_count) +static void mb_drv_dispatch_events(void *ctx) { port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); mb_event_info_t event_info; - while (event_count-- && (xQueueReceive(drv_obj->event_queue, &event_info, 0) == pdTRUE)) { + uint32_t event_count = 0; + while ((event_count < MB_EVENT_DISPATCH_MAX) + && (xQueueReceive(drv_obj->event_queue, &event_info, 0) == pdTRUE)) { + event_count++; mb_driver_event_num_t event_num = MB_EVENT_READY_NUM; while ((event_num < MB_EVENT_COUNT) && (MB_EVENT_FROM_NUM(event_num) != event_info.event_id)) { event_num++; @@ -535,14 +543,18 @@ void mb_drv_tcp_task(void *ctx) port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx); ESP_LOGD(TAG, "Start of driver task."); while (1) { + mb_drv_dispatch_events(ctx); + int wait_ms = uxQueueMessagesWaiting(drv_obj->event_queue) ? 0 : MB_SELECT_WAIT_MS; fd_set readset, errorset; FD_ZERO(&readset); FD_ZERO(&errorset); // check all active socket and fd events - int ret = mb_drv_wait_fd_events(ctx, &readset, &errorset, MB_SELECT_WAIT_MS); + int ret = mb_drv_wait_fd_events(ctx, &readset, &errorset, wait_ms); if (ret == ERR_TIMEOUT) { - // timeout occurred waiting for the vfds - DRIVER_SEND_EVENT(ctx, MB_EVENT_TIMEOUT, UNDEF_FD); + if (wait_ms > 0) { + // timeout occurred waiting for the vfds + DRIVER_SEND_EVENT(ctx, MB_EVENT_TIMEOUT, UNDEF_FD); + } mb_drv_check_suspend_shutdown(ctx); } else if (ret == -1) { // error occurred during waiting for vfds activation @@ -556,7 +568,6 @@ void mb_drv_tcp_task(void *ctx) uint64_t event_count = read_event(ctx); ESP_LOGD(TAG, "%p, event count: %" PRIu64, ctx, event_count); mb_drv_check_suspend_shutdown(ctx); - mb_drv_dispatch_events(ctx, event_count); } if ((drv_obj->listen_sock_fd >= 0) && FD_ISSET(drv_obj->listen_sock_fd, &readset)) { // If something happened on the listen socket, then it is an incoming connection. diff --git a/modbus/mb_ports/tcp/port_tcp_driver.h b/modbus/mb_ports/tcp/port_tcp_driver.h index 428c52ef..8b04aa99 100644 --- a/modbus/mb_ports/tcp/port_tcp_driver.h +++ b/modbus/mb_ports/tcp/port_tcp_driver.h @@ -43,6 +43,7 @@ typedef void (*mb_event_handler_fp)(void *ctx, void *data); #define MB_RX_QUEUE_MAX_SIZE (CONFIG_FMB_QUEUE_LENGTH) #define MB_TX_QUEUE_MAX_SIZE (CONFIG_FMB_QUEUE_LENGTH) #define MB_EVENT_QUEUE_SZ (CONFIG_FMB_QUEUE_LENGTH * MB_TCP_PORT_MAX_CONN) +#define MB_EVENT_DISPATCH_MAX (16) #define MB_DROP_TRANSACTION_TIME_US (1000UL * (CONFIG_FMB_TCP_KEEP_ALIVE_TOUT_SEC * 2000UL)) // drop after twice keep alive timeout is reasonable From 6e0d6a97c70213fc1bcf9e05e017de6c9fb71bb3 Mon Sep 17 00:00:00 2001 From: Andrey Gomanchuk Date: Tue, 4 Aug 2026 13:35:51 +0300 Subject: [PATCH 5/5] perf(tcp): send frames without writable precheck --- Kconfig | 5 +- modbus/mb_ports/tcp/port_tcp_master.c | 2 - modbus/mb_ports/tcp/port_tcp_utils.c | 70 ++++++++++++++++++--------- 3 files changed, 51 insertions(+), 26 deletions(-) diff --git a/Kconfig b/Kconfig index 014ec6a7..cf572a3c 100644 --- a/Kconfig +++ b/Kconfig @@ -77,8 +77,9 @@ menu "Modbus configuration" default 100 depends on FMB_COMM_MODE_TCP_EN help - This is a timeout for the socket writability check and send operation. - Before each send, the driver verifies the socket is writable via select function. + This is the total timeout for sending a complete Modbus TCP frame. + The driver sends without blocking first and waits for socket writability only + when the TCP send buffer cannot accept more data immediately. Reduce for faster failure detection; increase on high-latency links. config FMB_TCP_UID_ENABLED diff --git a/modbus/mb_ports/tcp/port_tcp_master.c b/modbus/mb_ports/tcp/port_tcp_master.c index 1bdd6779..e93afc22 100644 --- a/modbus/mb_ports/tcp/port_tcp_master.c +++ b/modbus/mb_ports/tcp/port_tcp_master.c @@ -615,8 +615,6 @@ MB_EVENT_HANDLER(mbm_on_send_data) mb_drv_lock(ctx); drv_obj->mb_node_curr = info_ptr; drv_obj->curr_node_index = info_ptr->index; - info_ptr->send_time = esp_timer_get_time(); - info_ptr->send_counter = (info_ptr->send_counter < (USHRT_MAX - 1)) ? (info_ptr->send_counter + 1) : 0; mb_drv_unlock(ctx); // Get send buffer from stack ESP_LOG_BUFFER_HEX_LEVEL("SENT", tx_buffer, sz, ESP_LOG_DEBUG); diff --git a/modbus/mb_ports/tcp/port_tcp_utils.c b/modbus/mb_ports/tcp/port_tcp_utils.c index 1697f1ed..99e25bd2 100644 --- a/modbus/mb_ports/tcp/port_tcp_utils.c +++ b/modbus/mb_ports/tcp/port_tcp_utils.c @@ -472,32 +472,58 @@ err_t port_connect(void *ctx, mb_node_info_t *info_ptr) int port_write_poll(mb_node_info_t *info_ptr, const uint8_t *frame, uint16_t frame_len, uint32_t timeout) { - if (frame_len > MB_TCP_BUFF_MAX_SIZE) { + if (!info_ptr || !frame || (info_ptr->sock_id < 0) || !frame_len + || (frame_len > MB_TCP_BUFF_MAX_SIZE)) { ESP_LOGE(TAG, MB_NODE_FMT(", refuse send: length %u > max %d"), - info_ptr->index, info_ptr->sock_id, info_ptr->addr_info.ip_addr_str, + info_ptr ? info_ptr->index : UNDEF_FD, + info_ptr ? info_ptr->sock_id : UNDEF_FD, + (info_ptr && info_ptr->addr_info.ip_addr_str) ? info_ptr->addr_info.ip_addr_str : "NULL", (unsigned)frame_len, MB_TCP_BUFF_MAX_SIZE); - return -1; - } - // Check if the socket is alive (writable and SO_ERROR == 0) - int ret = (int)port_check_alive(info_ptr, timeout); - if ((ret < 0) && (ret != ERR_INPROGRESS)) { - ESP_LOGE(TAG, MB_NODE_FMT(", is not writable, error: %d, errno %d"), - info_ptr->index, info_ptr->sock_id, info_ptr->addr_info.ip_addr_str, ret, (int)errno); - return ret; + return ERR_ARG; } - ret = send(info_ptr->sock_id, frame, frame_len, 0); - if (ret < 0) { - ESP_LOGE(TAG, MB_NODE_FMT(", send data error: %d, errno %d"), - info_ptr->index, info_ptr->sock_id, info_ptr->addr_info.ip_addr_str, ret, (int)errno); - info_ptr->error = ret; - } else { - ESP_LOG_BUFFER_HEX_LEVEL("SENT", frame, ret, ESP_LOG_DEBUG); - info_ptr->error = 0; - info_ptr->send_time = port_get_timestamp(); - info_ptr->send_counter = (info_ptr->send_counter < (USHRT_MAX - 1)) - ? (info_ptr->send_counter + 1) : 0; + + int64_t deadline_us = port_get_timestamp() + ((int64_t)timeout * 1000); + size_t sent_length = 0; + while (sent_length < frame_len) { + ssize_t ret = send(info_ptr->sock_id, &frame[sent_length], frame_len - sent_length, MSG_DONTWAIT); + if (ret > 0) { + sent_length += (size_t)ret; + continue; + } + if (ret == 0) { + info_ptr->error = ERR_CONN; + return ERR_CONN; + } + if (errno == EINTR) { + continue; + } + if ((errno != EAGAIN) && (errno != EWOULDBLOCK)) { + ESP_LOGE(TAG, MB_NODE_FMT(", send data error: %d, errno %d"), + info_ptr->index, info_ptr->sock_id, info_ptr->addr_info.ip_addr_str, + (int)ret, (int)errno); + info_ptr->error = ERR_CONN; + return ERR_CONN; + } + + int64_t remaining_us = deadline_us - port_get_timestamp(); + if (remaining_us <= 0) { + info_ptr->error = ERR_TIMEOUT; + return ERR_TIMEOUT; + } + uint32_t remaining_ms = (uint32_t)((remaining_us + 999) / 1000); + err_t err = port_check_alive(info_ptr, remaining_ms); + if (err != ERR_OK) { + info_ptr->error = (err == ERR_INPROGRESS) ? ERR_TIMEOUT : err; + return info_ptr->error; + } } - return ret; + + ESP_LOG_BUFFER_HEX_LEVEL("SENT", frame, frame_len, ESP_LOG_DEBUG); + info_ptr->error = ERR_OK; + info_ptr->send_time = port_get_timestamp(); + info_ptr->send_counter = (info_ptr->send_counter < (USHRT_MAX - 1)) + ? (info_ptr->send_counter + 1) : 0; + return frame_len; } // Scan IP address according to IPV settings