espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
13.31k stars 7.2k forks source link

Missing MB_TCP_UID in xMBMasterTCPPortSendResponse ( ) (IDFGH-4636) #6450

Closed ProKampas closed 2 years ago

ProKampas commented 3 years ago

Environment

Problem Description

I am implementing a Modbus TCP/IP application where I need to use the slave addresses [1, 2, 200]. Despite the issue I reported a few days ago (https://github.com/espressif/esp-idf/issues/6437), I came across another one. The problem lies in the function xMBMasterTCPPortSendResponse() in port_tcp_master.c and specifically in this part of the code (lines 900+):

    // If socket active then send data
    if (pxInfo->xSockId > -1) {
        // Apply TID field to the frame before send
        pucMBTCPFrame[MB_TCP_TID] = (UCHAR)(pxInfo->usTidCnt >> 8U);
        pucMBTCPFrame[MB_TCP_TID + 1] = (UCHAR)(pxInfo->usTidCnt & 0xFF);
        int xRes = xMBMasterTCPPortWritePoll(pxInfo, pucMBTCPFrame, usTCPLength, MB_TCP_SEND_TIMEOUT_MS);

When we send data using Modbus TCP/IP usually we don't need to include the Unit Identifier byte and we set it to 0. However, in applications where we need the slave address this byte needs to be set to the desired value. I searched throughout the entire Modbus software you provided and there isn't any set of the pucMBTCPFrame[MB_TCP_UID] value. Therefore, I propose, in the above piece of code to be added the following line:

        pucMBTCPFrame[MB_TCP_UID] = (UCHAR)(xMbPortConfig.ucCurSlaveIndex & 0xFF);

I checked the send and receive packets and the problem was that the MB_TCP_UID byte in the pucMBTCPFrame was always 0.

Steps to reproduce

  1. If you don't have a Modbus device ready, use a slave simulator and set a simple holding register with slave address !=0
  2. Create a trivial Modbus master TCP/IP application with the esp32 (maybe the example provided already in github) and try to get the value on that slave address of the slave.
  3. Print the send and receive data in port_tcp_master.c in (the end of vMBTCPPortMasterReadPacket() and in xMBMasterTCPPortWritePoll()
alisitsyn commented 3 years ago

Hi @ProKampas,

This current implementation of stack is Modbus TCP stack not Modbus RTU over TCP. The MB_TCP_UID for this implementation should be always 0 or 0xFF (Not Used). I can not confirm your issue and can not agree with your proposal because of this.

For your reference the Modbus spec says:

Unit Identifier

This field is used for routing purpose when addressing a device on a MODBUS+ or MODBUS serial line sub-network. In that case, the “Unit Identifier” carries the MODBUS slave address of the remote device: If the MODBUS server is connected to a MODBUS+ or MODBUS Serial Line sub-network and addressed through a bridge or a gateway, the MODBUS Unit identifier is necessary to identify the slave device connected on the subnetwork behind the bridge or the gateway. The destination IP address identifies the bridge itself and the bridge uses the MODBUS Unit identifier to forward the request to the right slave device. The MODBUS slave device addresses on serial line are assigned from 1 to 247 (decimal). Address 0 is used as broadcast address. On TCP/IP, the MODBUS server is addressed using its IP address; therefore, the MODBUS Unit Identifier is useless. The value 0xFF has to be used. When addressing a MODBUS server connected directly to a TCP/IP network, it’s recommended not using a significant MODBUS slave address in the “Unit Identifier” field. In the event of a re-allocation of the IP addresses within an automated system and if a IP address previously assigned to a MODBUS server is then assigned to a gateway, using a significant slave address may cause trouble because of a bad routing by the gateway. Using a nonsignificant slave address, the gateway will simply discard the MODBUS PDU with no trouble. 0xFF is recommended for the “Unit Identifier" as nonsignificant value. Remark : The value 0 is also accepted to communicate directly to a MODBUS/TCP device.

ProKampas commented 3 years ago

Hi @alisitsyn ,

I understand your point and thank you for your response. However, I would like to point out that Modbus RTU over TCP/IP is a protocol in use. And without many changes in your code structure you can add it in your codebase and give us the opportunity to use it. In my opinion, It's a feature that will be extremely useful and it requires minor changes in the software.

alisitsyn commented 3 years ago

Hi @ProKampas,

You are right. The Modbus stack architecture was designed to support all versions of protocol types. This implementation is planned and the code of TCP port layer designed to support this type of communication.

ProKampas commented 3 years ago

Hi @alisitsyn ,

Thank you. I am looking forward to your implementation.

alisitsyn commented 3 years ago

Hi @ProKampas,

This will take some time. I will consider your Issue as a feature request to implement the Modbus RTU over TCP.

alisitsyn commented 2 years ago

Hi @ProKampas,

Unfortunately the update is still not merged. The implemented fix can be found in the local component (mb_component folder) of the project stored here: https://github.com/alisitsyn/modbus_support/tree/mb_support/master_uid_gaps_rtu_over_tcp_support It will be possible to merge the fix soon.

alisitsyn commented 2 years ago

The setting UID in the MBAP has been merged into master with commit ID 2cad565781eb95ffbbe0840f801016acaca82e86 . It needs some time to sync with github.

Alvin1Zhang commented 2 years ago

Thanks for reporting, fix on master is available at https://github.com/espressif/esp-idf/commit/2cad565781eb95ffbbe0840f801016acaca82e86 and fix on release/4.3 is available at https://github.com/espressif/esp-idf/commit/5c78283f6eb9deea0b371851e318ea5f00675f14, feel free to reopen if the issue still happens, thanks.

crmabs commented 1 year ago

i recently bumped into an inverter that only works with a properly specified slave uid when using modbus tcp. if you need more details I can scrape the facts. each cheeky bastard ( modbus device ) has its own way. little differences but in this world an almost is a no. I got a nice collection of stories...

alisitsyn commented 1 year ago

Hi @crmabs ,

Can the kconfig option "FMB_TCP_UID_ENABLED" help you to communicate with your inverter? There are many different modbus devices with custom options and behavior that for example support UID = 0, and also other UIDs at the same time. Please tell your interesting stories here.... Thanks.

crmabs commented 1 year ago

I promise sharing those stories, but first I need a fix for this, and instead of fiddle with your code I drew a cartoon for you.

We are reading a Fronius Inverter Symo Gen24.

This is how I test the stuff with a neat command line tool. The green underlined parts are showing how I must address the device. From this approach I suspect there is a gateway which does the RTU over TCP. The command resulted a proper value. All good so far. modpoll_term_address

The op above got captured with Wireshark. The selected byte is the slave UID. After that the usual modbus data, cmd,reg,length. Ok, so this works. modpoll_wireshark_capture

The runtime log. I added a buffer dump line to see what gets sent. That's where the problem shows itself. esp_log

I haven't dived too deep into your lib's code. This was the first place where checked how do the configuration defines enable/disable code. Seems good to me.
mbtcp_m_looks_good

This is where I manually set up the slave uid - the part where my code can go wrong. usercode_tcpmaster_setup

These are the config default I'm using. modbus_related_sdkconfig

And this is the place where I injected that extra buffer print. injected_buffer_print

By the way about a year ago with idf4.x it worked like a charm.

Let me know if you need anything else! Many thanks for your help.

alisitsyn commented 1 year ago

As per your log and code the master assigns the UID field incorrectly as if the CONFIG_FMB_TCP_UID_ENABLED is not set, but it is set according to your sdkconfig. Then your slave drops the frame because of this. The functionality related to FMB_TCP_UID_ENABLED is tested before the merge and confirmed as functional. The issue can be observed due to incorrect FMB_TCP_UID_ENABLED configuration or if the incorrect version of the component is compiled instead of correct one. Please make sure you exclude the old component and use new one in your application. To exclude old component you need to include the line 'set(EXCLUDE_COMPONENTS freemodbus)' in your CMakeLists.txt file and use the manifest file like here. Please take a look to your *.map file to make sure if correct component is included into your project. I think you may be using the esp-idf v4.4 and do not exclude that component correctly. The generatated sdkconfig file should include the CONFIG_FMB_TCP_UID_ENABLED=y

The below is the log of TCP master with component esp-modbus v1.0.10. The "UID to send: 1" is what is actually set in the MBAP frame.

I (4050) wifi:AP's beacon interval = 102400 us, DTIM period = 1
I (4970) esp_netif_handlers: example_netif_sta ip: 192.168.88.252, mask: 255.255.255.0, gw: 192.168.88.1
I (4970) example_connect: Got IPv4 event: Interface "example_netif_sta" address: 192.168.88.252
I (4980) example_common: Connected to example_netif_sta
I (4980) example_common: - IPv4 address: 192.168.88.252,
I (4990) wifi:Set ps type: 0

I (4990) uart: ESP_INTR_FLAG_IRAM flag not set while CONFIG_UART_ISR_IN_IRAM is enabled, flag updated
Waiting IP0 from stdin:

I (16040) MASTER_TEST: IP(0) = [192.168.88.251] set from stdin.
Waiting IP1 from stdin:
I (23990) wifi:<ba-add>idx:0 (ifx:0, 64:d1:54:1a:23:5b), tid:0, ssn:5, winSize:64

I (24510) MASTER_TEST: IP(1) = [192.168.88.251] set from stdin.
Waiting IP2 from stdin:

I (33550) MASTER_TEST: IP(2) = [192.168.88.251] set from stdin.
I (33550) MASTER_TEST: IP(3) is not set in the table.
I (33570) MASTER_TEST: Configured 3 IP addresse(s).
I (33570) MASTER_TEST: Modbus master stack initialized...
I (33570) MB_TCP_MASTER_PORT: TCP master stack initialized.
I (33580) MB_TCP_MASTER_PORT: Host[IP]: "192.168.88.251"[192.168.88.251]
I (33580) MB_TCP_MASTER_PORT: Add slave IP: 192.168.88.251
I (33580) MB_TCP_MASTER_PORT: Host[IP]: "192.168.88.251"[192.168.88.251]
I (33600) MB_TCP_MASTER_PORT: Add slave IP: 192.168.88.251
I (33600) MB_TCP_MASTER_PORT: Host[IP]: "192.168.88.251"[192.168.88.251]
I (33610) MB_TCP_MASTER_PORT: Add slave IP: 192.168.88.251
I (33620) MB_TCP_MASTER_PORT: Connecting to slaves...
---I (33660) MB_TCP_MASTER_PORT: Connected 3 slaves, start polling...
I (34210) MASTER_TEST: Start modbus test...
I (34210) MB_TCP_MASTER_PORT: UID to send: 1
I (34240) MASTER_TEST: Characteristic #0 Data_channel_0 (Volts) value = 0.000000 (0x0) read successful.
I (34240) MB_TCP_MASTER_PORT: UID to send: 1
I (34270) MASTER_TEST: Characteristic #1 Humidity_1 (%rH) value = 0.000000 (0x0) read successful.
I (34280) MB_TCP_MASTER_PORT: UID to send: 1
I (34300) MASTER_TEST: Characteristic #2 Temperature_1 (C) value = 0.000000 (0x0) read successful.
I (34310) MB_TCP_MASTER_PORT: UID to send: 200
I (34360) MASTER_TEST: Characteristic #3 Humidity_2 (%rH) value = 0.000000 (0x0) read successful.
I (34370) MB_TCP_MASTER_PORT: UID to send: 200

Please check the recommendations above then clean and rebuild your project. Let me know if you still have problems with this.

Update: not related but please decrease your CONFIG_FMB_PORT_TASK_PRIO to lower value. The priority = 23, may prevent esp-timer to work correctly.

crmabs commented 1 year ago

Is it somehow possible to do the whole 'exclude freemodbus' raindance at component level? Just to be clear: the main CMakeLists.txt file must stay intact and I'd like to do all the extra dirs and exclude freemodbus on component level.

crmabs commented 1 year ago

image

Dudz, how come only these two are found? The first one is a proper tcp init for slave mode. What I'm missing is the master mode tcp init with similar code. Again, these are just first blinks on my side.

crmabs commented 1 year ago

image

This is what happens if I forcible change the input address before ...writePoll(... It tries to map that to the iptable..

alisitsyn commented 1 year ago

@crmabs ,

The first one is a proper tcp init for slave mode. What I'm missing is the master mode tcp init with similar code. Again, these are just first blinks on my side.

the master init is here master supports the list of slaves. The master takes the uid for slave from data dictionary. The ip address is defined in the slave_ip_address_table which is assigned using the field ip_addr in the communication structure. During the initialization master creates the list of slaves info where associates each slave uid with its IP address then uses it to connect to each slave. If the initialization of master is not completed correctly the master can not write to the slave which is not correctly registered in the port layer and is not connected.

The slave_uid in the communication structure is used only for slave and is dummy field for the master. In current implementation if you need to send data to different slave UID with the same IP you need to add the additional CID in data dictionary and add the same address in IP table for this CID.

alisitsyn commented 1 year ago

@crmabs ,

Is it somehow possible to do the whole 'exclude freemodbus' raindance at component level? Just to be clear: the main CMakeLists.txt file must stay intact and I'd like to do all the extra dirs and exclude freemodbus on component level.

It is possible to check the list of the components in esp-modbus cmake file and check if the freemodbus component is in the list because the esp-idf component dependencies are added earlier. However, the order of the components is not guaranteed and I can not rely on this in later esp-idf releases. Because of this I can not reliably override the freemodbus component or just exclude it from the list. What I definitely can do is to show the message if the freemodbus component is in the list of build components and should be excluded in the project cmake file. Even this may not work in later updates of esp-idf because the order of components in the list is not guaranteed. I would propose to do exclude of freemodbus on the project level to avoid possible issues. Also, this should not be a big issue.

patch component example