diff --git a/Kconfig b/Kconfig index 4d38df0..d558c7a 100644 --- a/Kconfig +++ b/Kconfig @@ -49,6 +49,49 @@ menu "Modbus configuration" The keep-alive time can be configured and should be set based on the actual needs of the application, considering the trade-off between keeping the connection alive and minimizing system load. + config FMB_TCP_EVENT_WAIT_MS + int "Modbus TCP event wait timeout (ms)" + range 10 500 + default 50 + depends on FMB_COMM_MODE_TCP_EN + help + This option represents a maximum time the driver task can block while waiting + for an event during each loop iteration. Lower values allow the system to handle failures + while waiting for socket IO events, though they slightly increase CPU usage. + For high-throughput applications with many concurrent connections, reducing to 10-20 ms may help. + + config FMB_TCP_READ_TIMEOUT_MS + int "Modbus TCP socket read timeout (ms)" + range 1 500 + default 50 + depends on FMB_COMM_MODE_TCP_EN + help + The option applies to SO_RCVTIMEO when reading the MBAP header from a socket. + This is the worst-case blocking time if a partial TCP segment arrives. + On a reliable LAN, 50-100 ms is sufficient. Lower values reduce the + maximum stall that one slow connection can impose on other connections. + + config FMB_TCP_SEND_TIMEOUT_MS + int "Modbus TCP send timeout (ms)" + range 10 1000 + 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. + 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 9ec555a..280de25 100644 --- a/modbus/mb_ports/tcp/port_tcp_driver.c +++ b/modbus/mb_ports/tcp/port_tcp_driver.c @@ -619,6 +619,7 @@ void mb_drv_tcp_task(void *ctx) } else { // 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", @@ -629,8 +630,10 @@ void mb_drv_tcp_task(void *ctx) if (err != ESP_OK) { ESP_LOGE(TAG, "%p, event loop run, returns fail: %x", ctx, (int)err); } - } else if (drv_obj->listen_sock_fd && FD_ISSET(drv_obj->listen_sock_fd, &readset)) { + } + if (drv_obj->listen_sock_fd && 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); mb_uid_info_t node_info; int sock_id = port_accept_connection(drv_obj->listen_sock_fd, &node_info); @@ -649,47 +652,45 @@ void mb_drv_tcp_task(void *ctx) } } } - } else { - // socket event is ready, process each socket event - mb_drv_check_suspend_shutdown(ctx); - int curr_fd = 0; - mb_node_info_t *node_ptr = NULL; - ESP_LOGD(TAG, "%p, socket event active: %" PRIx64, ctx, *(uint64_t *)&readset); - 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)) { - // 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); - if (ret > 0) { - ESP_LOGD(TAG, "%p, "MB_NODE_FMT(", frame received."), ctx, (int)node_ptr->fd, - (int)node_ptr->sock_id, node_ptr->addr_info.ip_addr_str); - mb_drv_lock(ctx); - node_ptr->recv_time = esp_timer_get_time(); - 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, - (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. - drv_obj->event_cbs.mb_sync_event_cb(drv_obj->event_cbs.port_arg, MB_SYNC_EVENT_RECV_FAIL); - ESP_LOGD(TAG, "%p, "MB_NODE_FMT(", frame error."), ctx, (int)node_ptr->fd, + } + // If socket events are ready, process each socket event + mb_drv_check_suspend_shutdown(ctx); + int curr_fd = 0; + 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)) { + // 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); + if (ret > 0) { + ESP_LOGD(TAG, "%p, "MB_NODE_FMT(", frame received."), ctx, (int)node_ptr->fd, + (int)node_ptr->sock_id, node_ptr->addr_info.ip_addr_str); + mb_drv_lock(ctx); + node_ptr->recv_time = esp_timer_get_time(); + 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, + (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. + drv_obj->event_cbs.mb_sync_event_cb(drv_obj->event_cbs.port_arg, MB_SYNC_EVENT_RECV_FAIL); + ESP_LOGD(TAG, "%p, "MB_NODE_FMT(", frame error."), ctx, (int)node_ptr->fd, + (int)node_ptr->sock_id, node_ptr->addr_info.ip_addr_str); + } else { + if (ret == ERR_CONN) { + ESP_LOGD(TAG, "%p, "MB_NODE_FMT(", connection lost."), ctx, (int)node_ptr->fd, (int)node_ptr->sock_id, node_ptr->addr_info.ip_addr_str); } else { - if (ret == ERR_CONN) { - ESP_LOGD(TAG, "%p, "MB_NODE_FMT(", connection lost."), ctx, (int)node_ptr->fd, - (int)node_ptr->sock_id, node_ptr->addr_info.ip_addr_str); - } else { - ESP_LOGD(TAG, "%p, "MB_NODE_FMT(", critical read error=%d, errno=%u."), ctx, (int)node_ptr->fd, - (int)node_ptr->sock_id, node_ptr->addr_info.ip_addr_str, (int)ret, (unsigned)errno); - } - DRIVER_SEND_EVENT(ctx, MB_EVENT_ERROR, node_ptr->index, ret); + ESP_LOGD(TAG, "%p, "MB_NODE_FMT(", critical read error=%d, errno=%u."), ctx, (int)node_ptr->fd, + (int)node_ptr->sock_id, node_ptr->addr_info.ip_addr_str, (int)ret, (unsigned)errno); } + DRIVER_SEND_EVENT(ctx, MB_EVENT_ERROR, node_ptr->index, ret); } - curr_fd++; - mb_drv_check_suspend_shutdown(ctx); } + curr_fd++; + mb_drv_check_suspend_shutdown(ctx); } } } diff --git a/modbus/mb_ports/tcp/port_tcp_driver.h b/modbus/mb_ports/tcp/port_tcp_driver.h index e860d98..42c7eae 100644 --- a/modbus/mb_ports/tcp/port_tcp_driver.h +++ b/modbus/mb_ports/tcp/port_tcp_driver.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -51,9 +51,9 @@ typedef void (*mb_event_handler_fp)(void *ctx, esp_event_base_t base, int32_t id #define MB_DROP_TRANSACTION_TIME_US (1000UL * (CONFIG_FMB_TCP_KEEP_ALIVE_TOUT_SEC * 2000UL)) // drop after twice keep alive timeout is reasonable #define MB_WAIT_DONE_MS (5000) -#define MB_SELECT_WAIT_MS (200) -#define MB_TCP_SEND_TIMEOUT_MS (500) -#define MB_TCP_EVENT_LOOP_TICK_MS (50) +#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, \ diff --git a/modbus/mb_ports/tcp/port_tcp_slave.c b/modbus/mb_ports/tcp/port_tcp_slave.c index bc1645f..a263ada 100644 --- a/modbus/mb_ports/tcp/port_tcp_slave.c +++ b/modbus/mb_ports/tcp/port_tcp_slave.c @@ -503,37 +503,22 @@ MB_EVENT_HANDLER(mbs_on_send_data) uint16_t msg_id = 0; int node_id = 0; (void)transaction_item_get_data(item, NULL, &msg_id, &node_id); - // Check if the TID is equal to the current received TID for this node. - // If not, means the slave was not able to process the previous transaction on time. - // The reason is too much active connections or incorrect response time or request rate in the master. - if ((node_id != pnode->index) || (tid != msg_id) || (tid != pnode->tid_counter) || (MB_GET_NODE_STATE(pnode) < MB_SOCK_STATE_CONNECTED)) { - uint64_t tick = (transaction_tick_t)transaction_item_get_tick(item); - uint64_t time_div_us = (esp_timer_get_time() - tick); - ESP_LOGD(TAG, "%p, " MB_NODE_FMT(", frame TID:0x%04" PRIx16 "!=0x%04" PRIx16 " ,%" PRIu64 " ,%" PRIu64 " ,%" PRIx16 ", slave is busy."), + // Check if the first queued transaction matches this response. + // Only a genuine cross-node mismatch or disconnected state should trigger + // the "slave busy" exception. The tid_counter comparison is not used here + // because it reflects the latest received TID, which may have been bumped + // by a master retry while the current transaction was still in-progress. + if ((node_id != pnode->index) || (tid != msg_id) || (MB_GET_NODE_STATE(pnode) < MB_SOCK_STATE_CONNECTED)) { + ESP_LOGD(TAG, "%p, " MB_NODE_FMT(", frame TID:0x%04" PRIx16 "!=0x%04" PRIx16 ", slave is busy."), ctx, (int)pnode->index, (int)pnode->sock_id, - pnode->addr_info.ip_addr_str, pnode->tid_counter, tid, esp_timer_get_time(), tick, msg_id); - ESP_LOGW(TAG, "%p, " MB_NODE_FMT(", handling time [ms]: %" PRIu64 ", exceeds slave response time in master."), - ctx, (int)pnode->index, (int)pnode->sock_id, - pnode->addr_info.ip_addr_str, (time_div_us / 1000)); - // Hard hack to respond to next transaction with an exception - // MB_TCP_MBAP_SET_FIELD(frame_entry.buf, MB_TCP_TID, pnode->tid_counter); + pnode->addr_info.ip_addr_str, tid, msg_id); // Build the exception frame to inform that slave is busy frame_entry.buf[MB_TCP_FUNC] = (frame_entry.buf[MB_TCP_FUNC] | 0x80); - frame_entry.buf[MB_TCP_LEN + 1] = 3; // Length: UID + FUNC + EXCEPTION + frame_entry.buf[MB_TCP_LEN + 1] = 3; frame_entry.buf[MB_TCP_FUNC + 1] = MB_EX_SLAVE_BUSY; ret = port_write_poll(pnode, frame_entry.buf, MB_TCP_FUNC + 2, MB_TCP_SEND_TIMEOUT_MS); mb_drv_lock(drv_obj); if (ret >= 0) { - err = transaction_set_state(port_obj->transaction, tid, TRANSMITTED); - if (err == ESP_OK) { - ESP_LOGD(TAG, "%p, " MB_NODE_FMT(", sent packet TID: 0x%04" PRIx16 ", %p."), - drv_obj, pnode->index, pnode->sock_id, - pnode->addr_info.ip_addr_str, tid, frame_entry.buf); - } else { - ESP_LOGE(TAG, "%p, " MB_NODE_FMT(", transaction set state fail for TID: 0x%04" PRIx16 ", %p."), - drv_obj, pnode->index, pnode->sock_id, - pnode->addr_info.ip_addr_str, tid, frame_entry.buf); - } if (transaction_delete(port_obj->transaction, tid) != ESP_OK) { ESP_LOGE(TAG, "Failed to remove queued TID:0x%04" PRIx16, tid); } else { @@ -545,6 +530,18 @@ MB_EVENT_HANDLER(mbs_on_send_data) (void)mb_drv_set_status_flag(drv_obj, MB_FLAG_TRANSACTION_READY); mb_drv_unlock(drv_obj); } else { + // Warn user if master sent a new request before this response was dispatched. + // The response is still sent normally — this only indicates that the master's + // response timeout is too short for the number of active connections. + uint64_t tick = (transaction_tick_t)transaction_item_get_tick(item); + uint64_t time_div_us = (esp_timer_get_time() - tick); + if (tid != pnode->tid_counter) { + ESP_LOGW(TAG, "%p, " MB_NODE_FMT(", handling time [ms]: %" PRIu64 ", exceeds slave response time in master, TID:0x%04" PRIx16 " != TID:0x%04" PRIx16), + ctx, (int)pnode->index, (int)pnode->sock_id, + pnode->addr_info.ip_addr_str, (time_div_us / 1000), + pnode->tid_counter, tid + ); + } ret = port_write_poll(pnode, frame_entry.buf, frame_entry.len, MB_TCP_SEND_TIMEOUT_MS); mb_drv_lock(drv_obj); if (ret >= 0) { diff --git a/modbus/mb_ports/tcp/port_tcp_slave.h b/modbus/mb_ports/tcp/port_tcp_slave.h index d9d6869..ca9e274 100644 --- a/modbus/mb_ports/tcp/port_tcp_slave.h +++ b/modbus/mb_ports/tcp/port_tcp_slave.h @@ -24,7 +24,7 @@ #if (CONFIG_FMB_COMM_MODE_TCP_EN) -#define TRANSACTION_TICKS pdMS_TO_TICKS(50) +#define TRANSACTION_TICKS pdMS_TO_TICKS(20) /** * @brief Modbus slave addr list item for the master diff --git a/modbus/mb_ports/tcp/port_tcp_utils.c b/modbus/mb_ports/tcp/port_tcp_utils.c index fe3ed78..3a1d228 100644 --- a/modbus/mb_ports/tcp/port_tcp_utils.c +++ b/modbus/mb_ports/tcp/port_tcp_utils.c @@ -245,7 +245,7 @@ int port_read_packet(mb_node_info_t *info_ptr) 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; diff --git a/modbus/mb_ports/tcp/port_tcp_utils.h b/modbus/mb_ports/tcp/port_tcp_utils.h index fb95554..3ab2468 100644 --- a/modbus/mb_ports/tcp/port_tcp_utils.h +++ b/modbus/mb_ports/tcp/port_tcp_utils.h @@ -75,7 +75,7 @@ extern "C" { #endif #define MB_MDNS_PORT (CONFIG_FMB_TCP_PORT_DEFAULT) -#define MB_READ_TICK (500) +#define MB_READ_TICK (CONFIG_FMB_TCP_READ_TIMEOUT_MS) #define MB_MDNS_QUERY_TIME_MS (2000) #define MB_STR_LEN_HOST 1 // "mb_node_tcp_01"