If optimize for performance in menuconfig, modbus failed on incomplete message (IDFGH-2371)

[keelung-yang]

On latest master. Optimize for size works, but if set to Optimize for performance, I got:

Modbus Error: [Input/Output] Modbus Error: [Invalid Message] Incomplete message received, expected at least 2 bytes (0 received)

Just change sdkconfig, no source code modified.

I'm using baudrate 230400, and I tried 115200 but still failed on the same error. ModbusSerialClient(strict=False or True) also have no effect.

Here is my test script in Python3(>=3.6):

import time

import serial

from pymodbus.client.sync import ModbusSerialClient

def reset(port, wait=1):
        with serial.Serial(port) as ser:
                ser.dtr = False
                ser.rts = False
        time.sleep(wait)

port = 'COM32'
baudrate=230400

reset(port)

client = ModbusSerialClient(
        method="rtu", strict=False,
        port=port, baudrate=baudrate, bytesize=8, stopbits=1, parity='N')
client.connect()

unit = 1
res = client.write_registers(0, [500], unit=unit)
if res.isError():
        print(f'{res}')
else:
        print('OK')

client.close()

sdkconfig and esp32 debug log also attached: SDKConfig & Log.zip

[keelung-yang]

@alisitsyn But why? It's not solved.

[alisitsyn]

@keelung-yang,

The issue you mentioned is about uart-echo example and uart driver in master branch. It is not about freemodbus component itself and I propose to open new issue.

[keelung-yang]

@alisitsyn OK then. I'll split it into uart and freemodbus issues, and rewrite reoccurable scripts/steps/setting ... on master branch, once it updated in this week.

[alisitsyn]

@keelung-yang,

Ok. Thanks.

[alisitsyn]

Hi @keelung-yang, The merge of the fixes we discussed will take more time than expected because of some dependencies. I will let you know when it is ready for merge.

[alisitsyn]

Hi @keelung-yang,

The fixes for issues discussed here have been merged. Could you check it on your side? In case it still exists please reopen separate issues against UART performance, freemodbus or something else. Thanks.

[keelung-yang]

@alisitsyn Optimizing for performance setting has no infinite reboot loop now on my board. But sending modbus message still get incomplete message in PyModbus (board rebooted I guess). I'm not sure whether the cause is in my app or esp-idf.

I think I need to reproduce it on official examples before firing a new issue, but I'm on other projects in full-time. I'll dig this once released from other projects. If reproducable issuse still exists then I'll fire a new issue.

Thank you very very much!

[alisitsyn]

@keelung-yang,

I will try to reproduce your issue on my side little bit later. I will be waiting for your test results as well.

Thank you for your feedback and feel free to open new issues.

[keelung-yang]

@alisitsyn I tried on my simplified modbus test app. It still failed on optimizing for performance setting.

C:\dev\esp\PicoBox\test\modbus>test.py -p COM5
Namespace(addr=0, baudrate=230400, data=4660, n=56, port='COM5', strict=False, unit=1)
Write Error: Modbus Error: [Input/Output] Modbus Error: [Invalid Message] Incomplete message received, expected at least 2 bytes (0 received)
Read Error: Modbus Error: [Input/Output] No Response received from the remote unit/Unable to decode response

As shown in output of test.py and the caption of this issue, we are going back to the beginning. Here is the source code to reproduct it and the console log: test_modbus.zip putty.log

[alisitsyn]

@keelung-yang,

Thank you for information. I was able to reproduce this. The issue is related to side effects when access FIFOs. The required fix for this was dropped from my MR because it was implemented in updated version of 2020r2 toolchain (compiler inserts memw instrruction as it was in patch above). I am working on this and will let you know as soon as possible.

[alisitsyn]

Update for the issue: I was able to confirm this bug using official uart_events example. The actual reason for the failure is that uart driver uart_hal_read_rxfifo() reads the UART FIFO using DPORT address. Actual cause: The packet sent: 0x01 0x03 0x00 0x00 0x00 0x01 0x84 0x0A The packet received from FIFO: 0x01 0x01 0x03 0x00 0x00 0x00 0x01 0x84 0x00 One duplicated byte is inserted at first position when reading the FIFO. This causes the Modbus driver to skip the packet as incorrect. The additional "memw" instruction fixes the issue as below: components/soc/esp32/include/hal/uart_ll.h:

static inline void uart_ll_read_rxfifo(uart_dev_t *hw, uint8_t *buf, uint32_t rd_len)
{
    //Get the UART APB fifo addr. Read fifo, we use APB address
    uint32_t fifo_addr = (hw == &UART0) ? UART_FIFO_REG(0) : (hw == &UART1) ? UART_FIFO_REG(1) : UART_FIFO_REG(2);
    for(int i = 0; i < rd_len; i++) {
        __asm__ __volatile__("memw");
        buf[i] = READ_PERI_REG(fifo_addr);
    }
}

The bug happens only when CONFIG_ESP32_DEFAULT_CPU_FREQ_240=y and CONFIG_COMPILER_OPTIMIZATION_PERF=y, otherwise the UART works just fine. I think this is access desynchronization issue between the DPORT which uses CPU frequency and AHB bus (uses different frequency). The additional "memw" instruction adds additional cycles to synchronize access. Please try the above fix and try to set the CPU frequency to 160Mhz. Any of these options should solve the issue. Please let me know the result. Thanks.

[keelung-yang]

@alisitsyn I've test __asm__ __volatile__("memw"); both in demo app test_modbus.zip posted here, and a more complicated application in a real world, downloading ECU by ISO14229-3 UDSonCAN by modbus from PC to ESP32 to ECU.

This one line patch works! THANKS!

[alisitsyn]

@keelung-yang, Thank you for the test results. I am glad that it works for you. The issue with UART FIFO will be investigated by digital team and documented later. As a temporary fix the nop.n instructions will be added to add additional delay cycles. Please feel free to open new issues in separate areas once you find them.

[AxelLin]

The uart fix is in 3a51584, maybe this issue should be closed?

[Alvin1Zhang]

Thanks for reporting and sharing the notes @AxelLin , feel free to reopen.