Apollo3zehn / FluentModbus

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

Endianess for ModbusTcpServer #31

Closed mkroesen closed 4 years ago

mkroesen commented 4 years ago

I have seen that the ModbusTcpServer checks if we have Little or Big-Endian. My system (Linux ARM32) is Little-Endian. When I check the data (with ModbusPoll) - it is not reversed.

For example:

Span<short> inputreg = server.GetInputRegisterBuffer<short>();
inputreg[0] = 1;

In ModbusPoll I got "256" (0x0100), I expected "1" (0x0001). It was possible that I manually reverse my data before with ModbusUtils.SwitchEndianness(), but I was wondering if there is a cleaner solution for it.

With the client I was able to receive the data in the correct endianess:

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

It would be great if someone could help me, or tell me if it makes sense to reverse every single value before.

Apollo3zehn commented 4 years ago

Where did you see that ModbusTcpServer checks endianness? It should be the client that does the checks.

I am wondering why client.Connect(..., ModbusEndianness.BigEndian) gives you the correct data with the server on a little endian system. Die you reverse the data in that test, too?

In general, do you have a preferred solution to work with big endian in the server more easily? I have thought about it, but did not find a good solution. On protocol level, Modbus uses INT16 big-endian registers. But on the user level I try to provide an abstraction to interpret the data as other types like INT8, vor FLOAT32 and others.

How would you provide these data to the server's register buffer in a generic way that is in the end easier than the SwitchEndianness method? I am happily accepting any good suggestions.

Maybe an extension method would work to use it like:

var buffer = server.GetInputRegisterBuffer<short>();
buffer.SetValue(index, value, endianess);

Would that suit your needs?

mkroesen commented 4 years ago

I saw it here: Server/ModbusRequestHandler.cs#L195

I did not reversed the data on reading, client.Connect(..., ModbusEndianness.BigEndian) was enough to receive correct data.

In C# I work with Int16 (short), but these are already reversed correct (for modbus client). I write e.g. 10 values into the InputRegisterBuffer (adress 0-9), in this case all values are inverted (incorrect) while reading with ModbusPoll. Actually I am writing the data in an extra short[], let it run through a loop (to reverse each value) and then assign it to the InputRegisterBuffer.

Span<short> data = _server.GetInputRegisterBuffer<short>();
lock (_server.Lock)
{
  Span<short> tmp = new short[50];
  // version
  tmp[0] = 1;
  tmp[1] = 0;
  tmp[2] = 0;
  // toggle bit
  tmp[3] = (short)(_toggleBit ? 1 : 0);
  // ...
  // ...
  // ...
  // tmp to data
  for (int i = 0; i < tmp.Length; i++) data[i] = SE(tmp[i]);
  tmp = null;
}

public static short SE (short value)
{
  var bytes = BitConverter.GetBytes(value);
  return (short)((bytes[0] << 8) + bytes[1]);
}

I assumed that with Server/ModbusRequestHandler.cs#L195 this happens automatically, but it does not work for me.

An extension method would have the disadvantage that I would have to specify it again for every single value, if there would be a global flag, e.g. in the constructor, would be very helpful.

Thanks in advance.

Apollo3zehn commented 4 years ago

From your sample code I think what should work is the following:

var server = new ModbusTcpServer(ModbusEndianness.BigEndian);
var data = server.GetInputRegisterBuffer<short>();
data.SetValue(index, value);

with an extension method like (untested):

public void SetValue<T>(uint index, this Span<T> value)
{
   if (<system endianness does not match endianness in constructor>)
      value[index] = ModbusUtils.SwitchEndianness(value);
}

Maybe server.GetInputRegisterBuffer<T>() should be extended, so that you can specify the start offset, like:

// without start offset, data[0] is always located at byte positions (0*8) to (1*8 - 1) = 0-7 which 
// is equal to Modbus registers 0 to 3.
var data = server.GetInputRegisterBuffer<double>();
data[0] = 24.2323;

// However, if you want to have the double value located at byte positions 2-10, 
// i.e. Modbus registers 1-4, you need a start offset for the span<double>:
var data = server.GetInputRegisterBuffer<double>(startOffset: 2);
data[0] = 24.2323;

Would this be a solution for your problem or is this somehow inconvenient to use?

Regarding https://github.com/Apollo3zehn/FluentModbus/blob/c9556463dd4185bca10e74c61b9ce965c8267172/src/FluentModbus/Server/ModbusRequestHandler.cs#L195 That just ensures that the protocol part has correct endianness, not the data part. It is impossible to ensure correct data endianness when it is unclear which data type the data actually has. E.g.:

On a little endian system, with bytes 0x01 0x02 0x03 0x04 the correct big endian representation of two int16 registers would be 0x02 0x01 0x04 0x03. And the big endian representation of a single int32 register would be 0x04 0x03 0x02 0x01;

Since the type is unknown, there is no way to reorder the bytes correctly. The only solution would be a map that assignes a data type to each buffer index (e.g. buffer<byte>[0-1] = short and buffer<byte>[2-5] = int. With that info byte reordering would work but there are more issues that come with that solution.

Therefore I think above solution with the extension method is not so bad. What do you think?

mkroesen commented 4 years ago

If I have understood everything correctly it looks like this: What you write into the buffer is returned 1:1 by the server on requests?

The idea with the extension method is good. What about performance if I process e.g. 50 values and in each SetValue an if-condition has to be checked?

With the startOffset it sounds quite good, but I have never used something like this before. I only work with Int16 and not with values that extend over several registers.

Would it not be enough to set the Double-Value to buffer[1] not [0], So I would have the offset after all?

Apollo3zehn commented 4 years ago

Yes the content is returned without changes. The check in the extension method will be lightweight, so you should not notice any performance impact.

Start offset: If you set the 8 byte double value to buffer[1], it would be located at byte positions 8-15 which is equal to Modbus registers 4-7 (with 2 bytes per Modbus register). But if you want to have it located at Modbus registers 1-5, you need a way to specify a start offset (in bytes).

Since everything is clear for me now, I will prepare a draft and hopefully finish it on Monday.

mkroesen commented 4 years ago

Sounds good, I think this is a good solution. Thanks a lot!