Big problem after running for some time using TCP Modbus Slave code (IDFGH-11297)

[jamesarm97]

I just updated to the latest version because I was having a weird issue with the previous version running a modbus tcp slave following the example code. Still happening with latest version. In a nutshell after a day or two, I start seeing everything that should be a HOLDING REGISTER READ coming in as a WRITE instead. I have updated and restarted the modbus poll app to make sure it isn't coming from that. The code is printing the operation using ESP_LOG right from the example and it seems that maybe the Event Group code in modbus is getting corrupt or out of sync or something because everything is coming in with the test (event & MB_EVENT_HOLDING_REG_WR) true even though the master side is calling a READ function not WRITE. Has anyone come across this? I am passing a lot of packets. It is like the event group is always setting the Write flag. Not sure where to look.

This is what the log is showing:

(this is the modbus poll tx data for the 19005 address: Tx:000128-00 78 00 00 00 06 01 04 4A 3D 00 01 ) the 04 4A 3D above is Read Input Register 19005 yet below I am receiving the code as a HOLDING WRITE so something is completely out of sync.

I (242949889) MB_SLAVE: HOLDING WRITE 81 (2429963452 us), ADDR:19005, TYPE:8, INST_ADDR:0x3ffb5fd4, SIZE:1 I (242950189) MB_SLAVE: HOLDING WRITE 81 (2430025214 us), ADDR:20210, TYPE:1, INST_ADDR:0x3ffb6416, SIZE:1

and this is the section of code printing the above:

        // Check for read/write events of Modbus master for certain events
        mb_event_group_t event = mbc_slave_check_event(MB_READ_WRITE_MASK);
        const char *rw_str = (event & MB_READ_MASK) ? "READ" : "WRITE";
        // Filter events and process them accordingly
        if (event & (MB_EVENT_HOLDING_REG_WR | MB_EVENT_HOLDING_REG_RD))
        {
            // Get parameter information from parameter queue
            ESP_ERROR_CHECK_WITHOUT_ABORT(mbc_slave_get_param_info(&reg_info, MB_PAR_INFO_GET_TOUT));
            ESP_LOGI(TAG, "HOLDING %s %02X (%lu us), ADDR:%lu, TYPE:%lu, INST_ADDR:0x%.4lx, SIZE:%lu",
                     rw_str, (uint8_t)event, (uint32_t)reg_info.time_stamp,
                     (uint32_t)reg_info.mb_offset,
                     (uint32_t)reg_info.type,
                     (uint32_t)reg_info.address,
                     (uint32_t)reg_info.size);
         }

[jamesarm97]

I did just see that the TYPE in the log printout is showing TYPE 8 which I just looked up and is MB_EVENT_INPUT_REG_RD so that part in the event structure seems right, just the event from reading mbc_slave_check_event(MB_READ_WRITE_MASK); is set with the WRITE set. Does a return from mbc_slave_check_event return read and write set if there are multiple events queued? I am not sure how this is possible unless one of them timed out on the master side since modbus should be process a packet at a time with a response. I only have one master running at this time. I could see if you had multiple masters trying to read and write at the same time.

[jamesarm97]

What is the proper way to use the mb_event_group_t event = mbc_slave_check_event(MB_READ_WRITE_MASK) function?

When you read it do you check for a read or a write then call the mbc_slave_get_param_info(&reg_info, MB_PAR_INFO_GET_TOUT) function to get the actual data then loop expecting only one flag to be set at a time? If you have both a read and a write flag get (is that possible), do you just call the mbc_slave_get_param_info(&reg_info, MB_PAR_INFO_GET_TOUT) function twice and it will return what is available each time? The example code just has it checked in a loop with one call to mbc_slave_get_param_info(&reg_info, MB_PAR_INFO_GET_TOUT) for every return of the mb_event_group_t event = mbc_slave_check_event(MB_READ_WRITE_MASK); call.

I ended up calling mbc_slave_get_param_info(&reg_info, MB_PAR_INFO_GET_TOUT) after the get group event returned and then tested the reg_info.type to do all the other testing and processing since I could not rely on the event returned to match.

[alisitsyn]

@jamesarm97 ,

This behavior can happen when the master is sending packets intensively and the processing task has significantly low priority compare to port task. The mbc_slave_check_event() which returns the event group mask of the queued events and may return two bits set together in the case if activation of the processing task is delayed or new parameter is received due to context switch during processing. In this case the correct event ( MB_EVENT_INPUT_REG_RD or MB_EVENT_INPUT_REG_WR) should be determined after call to mbc_slave_get_param_info(...) from the mb_param_info_t:type field of returned information structure. This function gets the information about the latest accessed parameter from the queue and is always actual for every parameter from the queue.

    for(;holding_reg_params.holding_data0 < MB_CHAN_DATA_MAX_VAL;) {
        // Wait for read/write events of Modbus master available in the queue (just clear the event group bits at exit)
        (void)mbc_slave_check_event(MB_READ_WRITE_MASK);
        // Get parameter information from parameter queue
        ESP_ERROR_CHECK_WITHOUT_ABORT(mbc_slave_get_param_info(&reg_info, MB_PAR_INFO_GET_TOUT));
        const char* rw_str = (reg_info.type & MB_READ_MASK) ? "READ" : "WRITE";

        // Filter events and process them accordingly
        if (reg_info.type & (MB_EVENT_HOLDING_REG_WR | MB_EVENT_HOLDING_REG_RD)) { // 
            ESP_LOGI(TAG, "HOLDING %s (%u us), ADDR:%u, TYPE:%u, INST_ADDR:0x%.4x, SIZE:%u",
                    rw_str,
                    (uint32_t)reg_info.time_stamp,
                    (uint32_t)reg_info.mb_offset,
                    (uint32_t)reg_info.type,
                    (uint32_t)reg_info.address,
                    (uint32_t)reg_info.size);
            // a variable that keeps the value of the action when being accessed
            uint32_t value32 = *(uint32_t*)reg_info.address;
..................

Let me know if you need more information.

[jamesarm97]

Thanks, that is what I kind of switched to doing while waiting for a response because I noticed the debug had the reg_info.type showing correctly. seems to be working so far.

[alisitsyn]

This possible issue is known long time ago and fixed in my other examples but for some reason I forgot to address it in official ones.

[alisitsyn]

Merged into master with commit 81aec7b929b9270dbb89143c930806e30fb5bd40.