Fix async unit

[mdavidsaver]

An attempt to fix #624, at least for asyncio. I'm not certain how unit ID should be handled here. Can a client have requests to multiple unit IDs in-flight? I've tried to account for this possibility by storing all unit IDs requested since the last (re)connect. If this isn't necessary, a simpler solution would be to only remember the most recently sent unit ID.

If this is seen as a reasonable approach, I will expand to the other async backends (although I'm only using/testing with asyncio).

[sonarqubecloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[dhoomakethu]

@mdavidsaver thanks for the PR and apologies for the delay. I am going to keep this in review for some more time. I do not have a strong opinion for storing the unit id's . This is something that a modbus application should handle which uses pymodbus as a driver and not pymodbus itself. Please correct me if I am not understanding this PR properly.

[janiversen]

I think there are a bigger misunderstanding here (Or it is in my head). UnitId in tcp is the same as slaveid in serial, so why do we want to keep track of that ???

I believe #624 is true but a whole different problem.

[janiversen]

According to the modbus protocol there can only be one request "in flight" at one connection. Please unitId addresses the slave (device) on the open connection, so we have to wait until a response is received before continuing.

We should try with a simpler solution. I can see that unitId is missing is some calls (at least in the documentation).

[tracernz]

According to the modbus protocol there can only be one request "in flight" at one connection

Is this still valid for Modbus TCP? That protocol specification equips the server to have multiple active transactions, and specifies "Several MODBUS transactions can be activated simultaneously on the same TCP Connection." It also includes the "Transaction Identifier" field in the MBAP header which would seem to facilitate this.

P.S. Great to see all the work you're doing lately 👍 .

[janiversen]

The server must be able to handle multiple transactions because it can have multiple clients connected, that is true for a number of the transport protocols. The server do not need to store the transactions since it is part of the request, which are used to form the response.

A client however connect to 1 server with a socket (tcp etc.) or a physical line, and can therefore only have one transaction outstanding. A client can support multiple devices using multiple sockets or multiple serial lines, but in that case each connection can use the same transaction. In our case 1 instance of modbusClient can only handle 1 connection so no need to store transactions.

[janiversen]

thanks for the nice words.

[tracernz]

These are the key passages from the "MODBUS Messaging on TCP/IP Implementation Guide V1.0b" I refer to:

A limit of 1 is no problem; the spec allows this, so I guess it's fine 😀 . In any case, I think this is a much bigger topic than the original issue referenced here, and the more I read the spec the more vague the descriptions appear 😆 .

[janiversen]

Your reference is correct, but you are referencing a gateway which is more correctly named a forwarder, this is a different topic, because that can e.g. have 1 server (tcp) and multiple clients (serial).

I have yet to see a end-device that allows multiple requests, nor do I understand the need for that....think about it send the following:

read_register()
write_register()

the device (server) might decide to do the write first...because there are no demand that transactions ids are handled in ascending sequence.

But for a forwarder the following example makes a lot of sense:

read_register(slave=1)
write_register(slave=2)
read_register(slave=3)

and the forwarder must be able to handle that.

We have hard coded limit=1, and no intention of changing that for the above reasons.

The description is not vague, you just need to see it together with the different device types.