Skip to content

Bugfix/fix master multiple errors handling #137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Bugfix/fix master multiple errors handling #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6d72bf1
fix master FSM error handling issue
alisitsyn Sep 18, 2025
3371343
fix logging
alisitsyn Sep 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/serial/mb_serial_master/sdkconfig.ci.ascii
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ CONFIG_FMB_TIMER_USE_ISR_DISPATCH_METHOD=y
CONFIG_FMB_EXT_TYPE_SUPPORT=y
CONFIG_HEAP_POISONING_COMPREHENSIVE=y
CONFIG_HEAP_TRACING_OFF=y
CONFIG_LOG_DEFAULT_LEVEL_DEBUG=y # intentionally keep it here to change when needed!

1 change: 1 addition & 0 deletions examples/serial/mb_serial_master/sdkconfig.ci.rtu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ CONFIG_FMB_TIMER_USE_ISR_DISPATCH_METHOD=y
CONFIG_FMB_EXT_TYPE_SUPPORT=y
CONFIG_HEAP_POISONING_COMPREHENSIVE=y
CONFIG_HEAP_TRACING_OFF=y
CONFIG_LOG_DEFAULT_LEVEL_DEBUG=y # intentionally keep it here to change when needed!
1 change: 1 addition & 0 deletions examples/serial/mb_serial_slave/sdkconfig.ci.ascii
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ CONFIG_FMB_TIMER_USE_ISR_DISPATCH_METHOD=y
CONFIG_FMB_EXT_TYPE_SUPPORT=y
CONFIG_HEAP_POISONING_COMPREHENSIVE=y
CONFIG_HEAP_TRACING_OFF=y
CONFIG_LOG_DEFAULT_LEVEL_DEBUG=y # intentionally keep it here to change when needed!
1 change: 1 addition & 0 deletions examples/serial/mb_serial_slave/sdkconfig.ci.rtu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ CONFIG_FMB_TIMER_USE_ISR_DISPATCH_METHOD=y
CONFIG_FMB_EXT_TYPE_SUPPORT=y
CONFIG_HEAP_POISONING_COMPREHENSIVE=y
CONFIG_HEAP_TRACING_OFF=y
CONFIG_LOG_DEFAULT_LEVEL_DEBUG=y # intentionally keep it here to change when needed!
2 changes: 1 addition & 1 deletion examples/tcp/mb_tcp_master/sdkconfig.ci.ethernet
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ CONFIG_FMB_MASTER_DELAY_MS_CONVERT=300
CONFIG_FMB_TCP_UID_ENABLED=n
CONFIG_FMB_TIMER_USE_ISR_DISPATCH_METHOD=y
CONFIG_MB_SLAVE_IP_FROM_STDIN=y
# CONFIG_LOG_DEFAULT_LEVEL_DEBUG=y # intentionally keep it here to change when needed
CONFIG_LOG_DEFAULT_LEVEL_DEBUG=y # intentionally keep it here to change when needed
CONFIG_FMB_EXT_TYPE_SUPPORT=y

CONFIG_EXAMPLE_CONNECT_IPV6=n
Expand Down
2 changes: 1 addition & 1 deletion examples/tcp/mb_tcp_master/sdkconfig.ci.wifi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ CONFIG_FMB_COMM_MODE_RTU_EN=n
CONFIG_FMB_COMM_MODE_ASCII_EN=n
CONFIG_FMB_MASTER_TIMEOUT_MS_RESPOND=3000
CONFIG_FMB_MASTER_DELAY_MS_CONVERT=300
# CONFIG_LOG_DEFAULT_LEVEL_DEBUG=y # intentionally keep it here to change when needed
CONFIG_LOG_DEFAULT_LEVEL_DEBUG=y # intentionally keep it here to change when needed
CONFIG_FMB_TCP_UID_ENABLED=n
CONFIG_MB_SLAVE_IP_FROM_STDIN=y
CONFIG_EXAMPLE_CONNECT_IPV6=n
Expand Down
2 changes: 1 addition & 1 deletion examples/tcp/mb_tcp_slave/sdkconfig.ci.ethernet
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ CONFIG_FMB_EXT_TYPE_SUPPORT=y
CONFIG_FMB_TCP_UID_ENABLED=n
CONFIG_FMB_TIMER_USE_ISR_DISPATCH_METHOD=y
CONFIG_MB_SLAVE_ADDR=1
# CONFIG_LOG_DEFAULT_LEVEL_DEBUG=y # intentionally keep it here to change when needed!
CONFIG_LOG_DEFAULT_LEVEL_DEBUG=y # intentionally keep it here to change when needed!
CONFIG_EXAMPLE_CONNECT_IPV6=n
CONFIG_EXAMPLE_CONNECT_WIFI=n
CONFIG_EXAMPLE_CONNECT_ETHERNET=y
Expand Down
2 changes: 1 addition & 1 deletion examples/tcp/mb_tcp_slave/sdkconfig.ci.wifi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ CONFIG_FMB_COMM_MODE_ASCII_EN=n
CONFIG_FMB_EXT_TYPE_SUPPORT=y
CONFIG_FMB_MASTER_TIMEOUT_MS_RESPOND=3000
CONFIG_FMB_MASTER_DELAY_MS_CONVERT=300
# CONFIG_LOG_DEFAULT_LEVEL_DEBUG=y # intentionally keep it here to change when needed!
CONFIG_LOG_DEFAULT_LEVEL_DEBUG=y # intentionally keep it here to change when needed!
CONFIG_FMB_TCP_UID_ENABLED=n
CONFIG_MB_SLAVE_ADDR=1
CONFIG_EXAMPLE_CONNECT_IPV6=n
Expand Down
10 changes: 5 additions & 5 deletions modbus/mb_objects/common/mb_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 3 additions & 4 deletions modbus/mb_objects/mb_master.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))) {
Expand All @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion modbus/mb_ports/common/port_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
38 changes: 28 additions & 10 deletions modbus/mb_ports/common/port_event.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,37 +161,55 @@ void mb_port_event_res_release(mb_port_base_t *inst)
}
}

void mb_port_event_set_resp_flag(mb_port_base_t *inst, mb_event_enum_t event_mask)
void mb_port_event_set_resp_flag(mb_port_base_t *inst, mb_err_event_t event_mask)
{
MB_RETURN_ON_FALSE((inst), ;, TAG, "incorrect object handle.");
(void)xEventGroupSetBits(inst->event_obj->event_group_hdl, event_mask);
(void)xEventGroupSetBits(inst->event_obj->event_group_hdl, (EventBits_t)event_mask);
}

mb_err_enum_t mb_port_event_wait_req_finish(mb_port_base_t *inst)
{
MB_RETURN_ON_FALSE((inst), MB_EINVAL, TAG,
"incorrect object handle.");
mb_err_enum_t err_status = MB_ENOERR;
mb_event_enum_t rcv_event;
EventBits_t bits = xEventGroupWaitBits(inst->event_obj->event_group_hdl,// The event group being tested.
MB_EVENT_REQ_MASK, // The bits within the event group to wait for.
pdTRUE, // Masked bits should be cleared before returning.
pdFALSE, // Don't wait for both bits, either bit will do.
MB_EVENT_QUEUE_TIMEOUT_MAX);// Wait forever for either bit to be set.
rcv_event = (mb_event_enum_t)(bits);
mb_err_enum_t err_status = MB_ETIMEDOUT;
mb_err_event_t rcv_event;
EventBits_t bits = EV_ERROR_INIT;
bits = xEventGroupWaitBits(inst->event_obj->event_group_hdl, // The event group being tested.
MB_EVENT_REQ_MASK, // The bits within the event group to wait for.
pdTRUE, // Masked bits should be cleared before returning.
pdFALSE, // Don't wait for both bits, either bit will do.
MB_EVENT_QUEUE_TIMEOUT_MAX); // Wait forever for either bit to be set.
rcv_event = (mb_err_event_t)(bits);
if (rcv_event) {
ESP_LOGD(TAG, "%s, %s: returned event = 0x%x", inst->descr.parent_name, __func__, (int)rcv_event);
if (!(rcv_event & MB_EVENT_REQ_MASK)) {
// if we wait for certain event bits but get from poll subset
ESP_LOGE(TAG, "%s, %s: incorrect event set = 0x%x", inst->descr.parent_name, __func__, (int)rcv_event);
}
if (MB_PORT_CHECK_EVENT(rcv_event, EV_ERROR_OK)) {
// Just to check if abnormal state is detected (multiple errors are active). Should not happen in normal FSM handling.
if (MB_PORT_CHECK_EVENT(rcv_event, (EV_ERROR_RECEIVE_DATA | EV_ERROR_RESPOND_TIMEOUT | EV_ERROR_EXECUTE_FUNCTION))) {
ESP_LOGD(TAG, "%s, %s: multiple errors detected? = 0x%x, clear.", inst->descr.parent_name, __func__, (int)rcv_event);
MB_PORT_CLEAR_EVENT(rcv_event, (EV_ERROR_RECEIVE_DATA | EV_ERROR_RESPOND_TIMEOUT | EV_ERROR_EXECUTE_FUNCTION));
}
err_status = MB_ENOERR;
} else if (MB_PORT_CHECK_EVENT(rcv_event, EV_ERROR_RESPOND_TIMEOUT)) {
if (MB_PORT_CHECK_EVENT(rcv_event, (EV_ERROR_RECEIVE_DATA | EV_ERROR_OK | EV_ERROR_EXECUTE_FUNCTION))) {
ESP_LOGD(TAG, "%s, %s: multiple errors detected? = 0x%x, clear.", inst->descr.parent_name, __func__, (int)rcv_event);
MB_PORT_CLEAR_EVENT(rcv_event, (EV_ERROR_RECEIVE_DATA | EV_ERROR_OK | EV_ERROR_EXECUTE_FUNCTION));
}
err_status = MB_ETIMEDOUT;
} else if (MB_PORT_CHECK_EVENT(rcv_event, EV_ERROR_RECEIVE_DATA)) {
if (MB_PORT_CHECK_EVENT(rcv_event, (EV_ERROR_RESPOND_TIMEOUT | EV_ERROR_OK | EV_ERROR_EXECUTE_FUNCTION))) {
ESP_LOGD(TAG, "%s, %s: multiple errors detected? = 0x%x, clear.", inst->descr.parent_name, __func__, (int)rcv_event);
MB_PORT_CLEAR_EVENT(rcv_event, (EV_ERROR_RESPOND_TIMEOUT | EV_ERROR_OK | EV_ERROR_EXECUTE_FUNCTION));
}
err_status = MB_ERECVDATA;
} else if (MB_PORT_CHECK_EVENT(rcv_event, EV_ERROR_EXECUTE_FUNCTION)) {
if (MB_PORT_CHECK_EVENT(rcv_event, (EV_ERROR_RECEIVE_DATA | EV_ERROR_OK | EV_ERROR_RESPOND_TIMEOUT))) {
ESP_LOGD(TAG, "%s, %s: multiple errors detected? = 0x%x, clear.", inst->descr.parent_name, __func__, (int)rcv_event);
MB_PORT_CLEAR_EVENT(rcv_event, (EV_ERROR_RECEIVE_DATA | EV_ERROR_OK | EV_ERROR_RESPOND_TIMEOUT));
}
err_status = MB_EILLFUNC;
}
} else {
Expand Down
6 changes: 2 additions & 4 deletions modbus/mb_ports/serial/port_serial.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down
10 changes: 5 additions & 5 deletions modbus/mb_ports/tcp/port_tcp_master.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
2 changes: 0 additions & 2 deletions test_apps/test_common/mb_utest_lib/port_adapter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down