Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert endianness #2506

Merged
merged 10 commits into from
Dec 18, 2024
Merged

Convert endianness #2506

merged 10 commits into from
Dec 18, 2024

Conversation

daanwtb
Copy link
Contributor

@daanwtb daanwtb commented Dec 17, 2024

No description provided.

Copy link
Collaborator

@janiversen janiversen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please read the standard:

'''
4.2 Data Encoding
 MODBUS uses a ‘big-Endian’ representation for addresses and data items. This means
that when a numerical quantity larger than a single byte is transmitted, the most significant
byte is sent first. So for example
Register size value
16 - bits 0x1234 the first byte sent is 0x12 then 0x34
'''

Your pull request would make sense, for int32 etc. that is changing the order of the registers (words) according to "little" or "big".

) -> int | float | str | list[bool]:
"""Convert registers to int/float/str.

:param registers: list of registers received from e.g. read_holding_registers()
:param data_type: data type to convert to
:param endian: endianness of the byte order
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong, byte order is always big.

What is not defined by the standard is the word order.

:returns: int, float, str or list[bool] depending on "data_type"
:raises ModbusException: when size of registers is not 1, 2 or 4
"""
byte_list = bytearray()
for x in registers:
byte_list.extend(int.to_bytes(x, 2, "big"))
byte_list.extend(int.to_bytes(x, 2, endian))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is wrong, a register is always "big".

@@ -725,12 +726,13 @@ def convert_from_registers(

@classmethod
def convert_to_registers(
cls, value: int | float | str | list[bool], data_type: DATATYPE
cls, value: int | float | str | list[bool], data_type: DATATYPE, endian: Literal["big", "little"] = "big"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above.

) -> list[int]:
"""Convert int/float/str to registers (16/32/64 bit).

:param value: value to be converted
:param data_type: data type to be encoded as registers
:param endian: endianness to encode bytes to
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above.

@@ -749,7 +751,7 @@ def convert_to_registers(
else:
byte_list = struct.pack(f">{data_type.value[0]}", value)
regs = [
int.from_bytes(byte_list[x : x + 2], "big")
int.from_bytes(byte_list[x : x + 2], endian)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above.

@daanwtb
Copy link
Contributor Author

daanwtb commented Dec 17, 2024

You are right about the modbus protocol stating each register should be big-endian, but we have encountered situations where it was not. I like the new implementation convert_from_registers/convert_to_registers but we can only use it if we can also use little endian byte_order. Proposing a new solution and like to hear your opinion.

@janiversen
Copy link
Collaborator

janiversen commented Dec 17, 2024

Pymodbus implements the modbus standard NOT all the non-standard implementations!

Apart from that it is hard to believe any devices would change the byte order, because it would pretty much break compatibility with every professional modbus software.

If you want a different byte order then please convert the bytes in the app.

Again the word order is different, because that is only a de facto standard so that could/should be open in pymodbus.

Copy link
Collaborator

@janiversen janiversen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You miss test cases that shows these conversions actually works.

) -> int | float | str | list[bool]:
"""Convert registers to int/float/str.

:param registers: list of registers received from e.g. read_holding_registers()
:param data_type: data type to convert to
:param byte_order: Literal[big, small] order of to bytes in 16 bit register encoding (ALMOST always big)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Byte order is fixed by the standard.

byte_list.extend(int.to_bytes(x, 2, "big"))
byte_list.extend(int.to_bytes(x, 2, byte_order))
if word_order == "little":
byte_list = byte_list[::-1]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not convert the order of all registers.

@janiversen janiversen merged commit f93b722 into pymodbus-dev:dev Dec 18, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants