Apollo3zehn / FluentModbus

Lightweight and fast client and server implementation of the Modbus protocol (TCP/RTU).
MIT License
211 stars 72 forks source link

Multi Unit RTU implementation #53

Closed aviatrix closed 3 years ago

aviatrix commented 3 years ago

Hello @Apollo3zehn!

The company I work for (Ondo Solutions) found your library quite useful and indispensable to our use cases. However, we found out few missing things that are critical for us, namely multiple unit support for RTU Modbus. So, we went ahead and added support for it in a separate class called MultiUnitRtuServer without moddifying the original RTU server. In the process we also found missing support forMidLittleEndian encoding which we also added just today so it resolves #45

There are few things to polish out, but i think the current state of the PR can qualify as a release candidate and we've been using it internally for about a month. There's no rush to merge this, so please take your time and share your thoughts and any concerns you might have.

Cheers!

Apollo3zehn commented 3 years ago

Thank you very much for your PR. Right before you published it, I went through your repository and had a quick look to the implementation. Could you please explain shorty, what a MultiUnitRtuServer does? Is it a quick way to have multiple servers behind a single COM port?

Thanks also for the MidLittleEndian solution.

I will review it soon and come back to you!

aviatrix commented 3 years ago

Is it a quick way to have multiple servers behind a single COM port?

Sort of. It's still a single instance of Modbus RTU server, but it makes use of the "Unit" feature of modbus RTU, which can handle 253/4 devices behind a single COM port (0/255 being broadcast, but not always ) Each unit is ( can be) a device, so now you can receive/send data from/to multiple devices

The implementation we've done has a separate register buffer for each unit, so now you can store data for 253 devices with a single RTU server :)

Apollo3zehn commented 3 years ago

Do I understand it right that a real-world setup with the MultiUnitRtuServer would look like something like this?

Unbenannt

I have never really worked with Modbus RTU, that's why I am asking. I try to understand the use case :-). In your specific case, does the RTU server run on a server or some embedded devices and these devices control one or more sensors or actors, so you need a full set of registers/buffers per sensor/actor?

aviatrix commented 3 years ago

Our usecase is simulating devices over RTU, so we have setup a virtual com port that forwards data from com1 to com2 on com 1 we have a RTU client and on com2 the RTU server, both the client and the server are on the same machine (windows/linux).
Our real physical setup has N devices behind single com port, so to create test env without physical devices, we created software devices, and we use the multi unit RTU server to simulate N devices and their behavior, which enables us to do integration tests and e2e tests without physical devices.

image

Apollo3zehn commented 3 years ago

Thanks, I got it now. In that case I think it makes sense to completely replace the current single unit RTU server. I will check that and see what needs to be changed. Do you mind if I push commits to this PR to make the changes required to be able to merge? These commits will also show up in the branch you used to create the PR.

aviatrix commented 3 years ago

Do you mind if I push commits to this PR to make the changes required to be able to merge?

Please do :)

Apollo3zehn commented 3 years ago

I have adapted and integrated your changes. I generalized your idea of a multi unit Modbus RTU server and extended the base class ModbusServer to support it. So any implementation that derives from that inherits the new feature. However, from my understanding, multiple units make no sense for simple Modbus TCP servers, so the default implementation does not support this. But if anybody wants to simulate TCP-RTU bridges or similar devices, he could implement it.

The API is nearly the same as in your original PR:

var server = new ModbusRtuServer(unitIdentifiers: new byte[] { 1, 2, 3 });
server.AddUnit(4);
server.GetCoilBuffer(unitIdentifier: 3);
server.RemoveUnit(1);

There are also the methods registers.SetMidLittleEndian<T>(...); and registers.GetMidLittleEndian<T>(...); based on your implementation (but changed a little for hopefully slightly better performance).

Could you please check if the changes I are correct and working for you? I have published a prerelease package on Nuget (https://www.nuget.org/packages/FluentModbus/4.0.0-preview.1.final).

Thank you :)

schotime commented 3 years ago

I'm having a similar / related issue but it might be an edge case to all of this when reading uint/ints The modbus server returns BigEndian, so I connect like so.

client.Connect(IPAddress.Parse("127.0.0.1"), ModbusEndianness.BigEndian);

I then read off two ushort/shorts

var result = await client.ReadInputRegistersAsync<ushort>(0x01, 3322, 2);

Now since I'm on a little endian architecture, the bytes get flipped for each ushort. These bytes are now in the correct order, as each set has been flipped. I have now adapted one of your GetLittleMidEndian as follows to get the value.

public static T GetMyLittleEndian<T>(this Span<ushort> buffer, int address)
    where T : unmanaged
{
    if (!(0 <= address && address <= ushort.MaxValue))
        throw new Exception();

    var byteBuffer = MemoryMarshal
        .AsBytes(buffer)
        .Slice(address * 2);

    var value = Unsafe.ReadUnaligned<T>(ref byteBuffer.GetPinnableReference());

    return value;
}

Now i'm not sure where this fits into this PR, but I thought I'd post my results. Let me know if this makes sense or there is already a built in way.

Apollo3zehn commented 3 years ago

@schotime I am not sure what the purpose of your implementation is. You said "These bytes are now in the correct order, as each set has been flipped". So if everything is fine, why do you need GetMyLittleEndian?

schotime commented 3 years ago

To convert 2 ushorts into an int.

Apollo3zehn commented 3 years ago

In that case you could directly read the data as int: var result = await client.ReadInputRegistersAsync<int>(0x01, 3322, 1);, dont't you?

schotime commented 3 years ago

That reads the bytes then swaps 1 with 4 and 2 with 3. Instead of 1 with 2 and 3 with 4, which I need it to be.

aviatrix commented 3 years ago

Please share the hex values of the two short registers separately, their order and the expected int/uint value

schotime commented 3 years ago

Two ushorts (raw bytes to follow). Expected number 100000.

1: 134, 160
2: 0, 1

My destination is BigEndian. So if I retrieve them as 2 ushorts then I get

1. 160, 134
2. 1, 0

as each pair is flipped for my little endian machine.

Then putting all those together.

var num = BitConverter.ToUInt32(new byte[] { 160, 134, 1, 0 }, 0)
Console.WriteLine(num); // 100000

However if I call the

var result = await client.ReadInputRegistersAsync<int>(0x01, 3322, 1);

then the bytes come out as

[1, 0, 160, 134]

due to the following being run.

public static void SwitchEndianness<T>(Span<T> dataset) where T : unmanaged
{
    var size = Marshal.SizeOf<T>();
    var dataset_bytes = MemoryMarshal.Cast<T, byte>(dataset);

    for (int i = 0; i < dataset_bytes.Length; i += size)
    {
        for (int j = 0; j < size / 2; j++)
        {
            var i1 = i + j;
            var i2 = i - j + size - 1;

            byte tmp = dataset_bytes[i1];
            dataset_bytes[i1] = dataset_bytes[i2];
            dataset_bytes[i2] = tmp;
        }
    }
}

which obviously is not correct.