espressif / esp-modbus

ESP-Modbus - the officially suppported library for Modbus protocol (serial RS485 + TCP over WiFi or Ethernet).
Apache License 2.0
100 stars 48 forks source link

How to clear RX buffer of MODBUS RTU (IDFGH-8193) #11

Closed abhi-ratnman closed 1 year ago

abhi-ratnman commented 2 years ago

Hi, I am trying to read MODBUS data using the above component.

The component works fine normally, but when there is some other data send between 2 polls the request considers the last random data received as the data for the request we sent now. Due to that the code is getting stuck. Is there any API or provision to clear data from RX buffer?

ESP IDF version : v4.4 Board : ESP32 WROOM32

abhi-ratnman commented 2 years ago

Hello any update on this? I tried debugging. When I send incorrect response in that case the MODBUS Master is stuck. Please find the attached image with debug enabled. The above parameter failed as there was no response. But in the below parameter I just send some random data, and due to that it stucked the code.

mb

alisitsyn commented 2 years ago

Hello abhi-ratnman ,

Do you use the freemodbus component of esp-idf v4.4 or esp-modbus? Please also send the sdkconfig file. Thank you.

abhi-ratnman commented 2 years ago

Using https://github.com/espressif/esp-modbus in the current project

SDK Config file :

alisitsyn commented 2 years ago

@abhi-ratnman ,

Thank you for the information. This is confirmed as a bug. This happens if the first byte of frame is 0x00 it is filtered but the timeout timer is disabled this is why the FSM did not get correct events triggered.

abhi-ratnman commented 2 years ago

@alisitsyn, thanks for a prompt reply.

What is the solution to this problem?

alisitsyn commented 2 years ago

@abhi-ratnman ,

Could you please fix the code of the component on your side as below and check how this fix works?

  1. In your project main component folder please remove the manifest file idf_component.yml.
  2. Create the components folder in the project directory.
  3. Move the espressif__esp-modbus component folder located in your project directory into components folder.
  4. Change the code starting from this line as below:
        xReadStatus = pxMBMasterPortCBTimerExpired();
        if (!xReadStatus) {
            xMBMasterPortEventPost(EV_MASTER_FRAME_RECEIVED);
            ESP_LOGI("RX_POLL", "Send additional RX ready event.");
        }
  5. idf.py fullclean then follow with idf.py build, idf.py -p PORT flash monitor This fix should solve the problem with one first zero byte in the frame. Please let me know the result of this quick fix. Then we can follow with your other questions. Thank you.
abhi-ratnman commented 2 years ago

@alisitsyn Yes, thanks. This fix works fine now when 0x00 is received as data no error is thrown.

Secondly what I am facing sometime is when I run the MODBUS for continuously long time at that time after some times what happens is data of parameter that is read before gets shifted. Below attached are images of the correct data received and shifted data received.

In the image below you can check that the data received above is correct (verify by temperature reading) and in the below data read the temperature reading is shifted.

14 38 correct 14 39 error

So basically once the shifted reading comes after that every time the readings are shifted until the module is manually restarted. So If I can erase the RX buffer of MODBUS so if such error occur once in the next reading such error does not occur.

abhi-ratnman commented 2 years ago

Hi @alisitsyn did you get what the error is or you need more details for the same?

alisitsyn commented 2 years ago

Hi @abhi-ratnman ,

I have got enough information.

The issue is known and I had different realizations to solve this but unfortunately it is hard to implement on the stack layer due to some design assumptions of freemodbus and because the driver needs to be universal and maintainable.

The issue mainly comes from Modbus protocol itself since it does not have any fields (in RTU and ASCII ADU frame) to check if the received response is correct and corresponds to the current transaction (request). The command and address fields are checked but it is not enough.

  1. The Master sends the request.
  2. The Master respond timed out and slave needs time for processing, the master starts the next request transaction.
  3. Slave responds after the Master sent the new request.
  4. The Master receives the data which corresponds to previous transaction and takes it into account (causes the data of read register is incorrect).
  5. Master starts the next transaction and receives the response of slave to previous request.
  6. The cycle is repeated and data is received with shifting.

The other similar scenarios are possible and in real the respond unfortunately can come at any time that prevents an effective filtering on stack layer. The filtering on user side can be more effective because the reasonable delay can be inserted there. So, to avoid the issue it could be enough to implement simple scenario like if the read/write API returns the error (ESP_ERR_INVALID_RESPONSE or ESP_ERR_TIMEOUT) the code waits for time proportional to CONFIG_FMB_MASTER_TIMEOUT_MS_RESPOND and clears the receive buffer before starting new transaction (call to read/write API).

abhi-ratnman commented 2 years ago

@alisitsyn thanks for the response. Which API to use to clear the receive buffer?

alisitsyn commented 2 years ago

@abhi-ratnman ,

The dedicated API is not yet provided. You can use the esp_err_t uart_flush_input(uart_port_t uart_num) for now but it does not clear the UART queue. I will allocate the public function similar to this.

abhi-ratnman commented 2 years ago

@alisitsyn i will need to clear the UART queue because currently what is happening is even if once the UART data is shifted, then it is shifted everytime, that is because the last data is stored in the queue while the slave give Correct response it reads the previous data in the queue. So whole queue needs to be cleared. How to do that?

What I plan to do is clear UART queue everytime before sending a request for reading, so in case any previous data was stored or received after timeout, we discard it and get the new data correct

alisitsyn commented 2 years ago

@abhi-ratnman ,

I understand this. The received data is stored in the UART ring buffer the uart_flush_input() could reset it. The queue stores just UART events and effectively needs to be cleared too. We don't know when data coming to the receiver and this may cause the fragmentation of the frames and in the legacy freemodbus realization can cause more serious problems. However I have some stack code that overrides the legacy ascii/rtu module and does what you mentioned - clears the buffer before the transaction. This can be more reliable and effective on user layer as I said and I would agree with your proposal and will add this API later. So, see the code for more information on how to implement this.

I would propose to use the function like below to wait during slave response time and discard the data and clear the event queue while buffer is not empty:

static void vMBMasterRxFlushTimeout( void )
{
    size_t xSize = 1;
    esp_err_t xErr = ESP_OK;
    int64_t xStartTime = esp_timer_get_time();
    int64_t xTimeLeft = 0;
    while( (xTimeLeft < (1000 * MB_MASTER_TIMEOUT_MS_RESPOND)) ){
        xTimeLeft = esp_timer_get_time() - xStartTime;
        xErr = uart_get_buffered_data_len(xPortContext.ucUartNumber, &xSize);
        MB_PORT_CHECK((xErr == ESP_OK), ; , "mb flush serial fail, error = 0x%x.", xErr);
        BaseType_t xStatus = xQueueReset(xPortContext.xMbUartQueue);
        if (xStatus) {
            xErr = uart_flush_input(xPortContext.ucUartNumber);
            MB_PORT_CHECK((xErr == ESP_OK), ; , "mb flush serial fail, error = 0x%x.", xErr);
        } 
    }
}

Let me know if you have any questions.

abhi-ratnman commented 2 years ago

@alisitsyn
Have been testing the code for some time now. I'm not using the timing to clear the buffer as per above. But I'm using the function you have to clear the buffer before any transaction. And testing for 24 hrs now and no no issues observed. Will test for 2 more days and if everything is fine will close the issue. Thank for your help

alisitsyn commented 1 year ago

@abhi-ratnman ,

Thank you for information. I am looking forward to get your results.

alisitsyn commented 1 year ago

Hi @abhi-ratnman ,

Do you have any update/results for the issue?

abhi-ratnman commented 1 year ago

In testing no issue was observed till now. Thank you for your help. I will close the issue

alisitsyn commented 1 year ago

@abhi-ratnman ,

Thank you for feedback. The function to reset input buffer will be included in the new version of component as well as description on how to use this.

Kakomis commented 11 months ago

@alisitsyn Have been testing the code for some time now. I'm not using the timing to clear the buffer as per above. But I'm using the function you have to clear the buffer before any transaction. And testing for 24 hrs now and no no issues observed. Will test for 2 more days and if everything is fine will close the issue. Thank for your help

Hi, I'm in the same situation, How did you solve it? Did you only use xErr = uart_flush_input(xPortContext.ucUartNumber); or the whole suggested function? Because otherwise, Where do you get xPortContext.xMbUartQueue?

alisitsyn commented 11 months ago

Hi @Kakomis,

In order to clear input buffer and UART queue to discard some data you can use the function vMBMasterRxFlush()`. This can be declared as:

extern void vMBMasterRxFlush( void );

void app_main(void)
{
    // Initialization of device peripheral and objects
    ESP_ERROR_CHECK(master_init());

    vMBMasterRxFlush(); // use the function in your code where you need to clear the input buffer

    master_operation_func(NULL);
}

Can it be used in your case? Let me know if you need more information.

Kakomis commented 11 months ago

Hi @alisitsyn thanks for your time, No, compiler says: undefined reference to `vMBMasterRxFlush', my code is based on the Modbus Master Example

alisitsyn commented 11 months ago

@Kakomis, It is strange, because it is checked on my side and can work. However, I don't know which version of the component you are using. In worse case you can change the component CMakeLists.txt file to expose whatever header you need.

.......................    
    "common/esp_modbus_slave_tcp.c"
    "common/esp_modbus_master_serial.c"
    "common/esp_modbus_slave_serial.c")

set(include_dirs common/include modbus/include port) # <<<<<<<<<<<<< Add the modbus/include port to public include path

set(priv_include_dirs common modbus modbus/ascii modbus/functions
                      modbus/rtu modbus/tcp)

list(APPEND priv_include_dirs serial_slave/port serial_slave/modbus_controller
     serial_master/port serial_master/modbus_controller
     tcp_slave/port tcp_slave/modbus_controller
     tcp_master/port tcp_master/modbus_controller)
....................
#include "port.h"
#include "mbport.h"

void app_main(void)
{
    // Initialization of device peripheral and objects
    ESP_ERROR_CHECK(master_init());

    vMBMasterRxFlush();

    master_operation_func(NULL);
}

Then you can include the mbport.h file which exposes the function. The same can be done a little bit harder through your project CMakeLists.txt file in order to keep the component.