mycroes/Sally7

Endianness of the data payload

Thieum opened this issue · 3 comments

This is more of a question than an issue regarding endianness of the data. I've checked both pages you link in your readme file for hints about that and didn't find anything regarding data. Both doc are more concerned about the headers, like if the data was left to the implementers to decide about the endianness.

My use case is 3 data block all holding a short, I implemented two ways to read them:

                var activeFaultsCalibration = new DataBlockDataItem<short> { DbNumber = 1, StartByte = 200 };

                var activeFaultsLearning = new DataBlockDataItem<short> { DbNumber = 1, StartByte = 202 };

                var activeFaultsSorting = new DataBlockDataItem<short> { DbNumber = 1, StartByte = 204 };

                var activeFaultsCalibrationArray = new DataBlockDataItem<byte[]> { DbNumber = 1, StartByte = 200,Length = 2 };

                var activeFaultsLearningArray = new DataBlockDataItem<byte[]> { DbNumber = 1, StartByte = 202, Length = 2 };

                var activeFaultsSortingArray = new DataBlockDataItem<byte[]> { DbNumber = 1, StartByte = 204, Length = 2 };

This is the data I receive from the server:

image

This is the code that treat the values:

                Console.WriteLine($"Active faults calibration: {activeFaultsCalibration.Value}");

                Console.WriteLine($"Active faults learning: {activeFaultsLearning.Value}");

                Console.WriteLine($"Active faults sorting: {activeFaultsSorting.Value}");

                short value = BitConverter.ToInt16(activeFaultsCalibrationArray.Value, 0);

                Console.WriteLine($"Active faults calibration Array: {value}");

                value = BitConverter.ToInt16(activeFaultsLearningArray.Value, 0);

                Console.WriteLine($"Active faults learning Array: {value}");

                value = BitConverter.ToInt16(activeFaultsSortingArray.Value, 0);

                Console.WriteLine($"Active faults sorting Array: {value}");

and this is what get displayed:

image

The data is little endian, but the library is opiniated as big-endian for the data. I control the code of the server, so it's not an issue to do the conversion either on the server or the client.

I was wondering if there was something in the specs that defined this behavior? Or is it something that should be optional somehow if it's left to interpretation?

It's really not a big deal though, as your library exposes enough way to circumvent this.

The outro of the linked articles gives a hint that it might just be a weakness of the protocol:

This might be obvious now, but the S7 protocol is not a well designed one. It was originally created to simply query register values, which it did kind of all right, but then functionality was kept being added until it became this monstrosity. It is filled with inconsistencies and unnecessary redundancies and it only gets worse with Userdata messages. These irregularities and design flaws become way more obvious (and annoying) while trying to write a parser for the protocol.

But I wanted your opinion on this, as your library seems to be opiniated on this particular subject.

Hi @Thieum,

According to the Siemens SAPI-S7 docs, page 211 (I'm not sure if this file is legally there, I have a legal digital copy of that exact document though) data in the PLC is big endian. The conversion in Sally7 actually doesn't care about the host system, while BitConverter actually expects bytes in order matching to the host order, which on X86 is little endian.

To explain that in more depth, if you read two bytes from the PLC, which is configured as a WORD or INT (16 bit), they'll be in big endian order, meaning the most significant byte is the first of the two. If you then convert that to a 16 bit number using BitConverter.ToInt16, the numeric representation will be wrong. However, if you tell Sally7 you want to read a short / Int16 or ushort / UInt16 then Sally7 will convert it to the appropriate numeric value. Sally7 uses simple bit shifts and binary or's to achieve this, which is actually faster than using BitConverter and also doesn't depend on the endianness of the host.

Besides having this properly supported for WORD and INT, this works for any type up to 8 bytes / 64 bits in length (as far as I know there's a thing called LINT which is the S7 counterpart of a C# long, either way if you have a big endian 64bit number in the PLC it will properly read/write it). That means it also works for float, or enums backed by byte, short, int or long or their unsigned counterparts. Last but not least it also works for arrays of all these types.

You can find the implementation of this logic in FromS7Conversions.cs. There actually is another solution in Sally7 to achieve the WORD conversion: BigEndianShort. This struct is used in the larger protocol structs, because I want to avoid conversions on the protocol information. It has the same conversions as are used in the value converter classes, but it also supports implicit int and byte casts because that avoids some casts in other places in the code.

Long story short, if you mimic the Siemens data types by their C# counterparts in the DataBlockDataItems, you don't need to do any conversion.

Thanks for all the info. It gave me arguments to make the server send Big Endian values, and drop the conversion, as suggested.

You're welcome. I wasn't trying to convince you to change the endianness, just explained the underlying reasons for the current implementation.