search

AdvancedClimateSystems / uModbus

Python implementation of the Modbus protocol.
Mozilla Public License 2.0
211 stars 82 forks source link

Easy data packing and unpacking (Write and Read) with the methods data_packer & data_unpacker #66

Closed rtu-dataframe closed 5 years ago

rtu-dataframe commented 5 years ago

@OrangeTux, i've added the packer methods to keep simple the data reading and writing, as requested in the issue #61 , i've created three functions in the utils.py in order to simplify the data packing and unpacking while reading/writing some different data types like Float, Double, etc... (Data Types Here)

In everyday use, you can simply use those methods importing the utils.py module and read (for example) a float value like shown below:

f = data_unpacker(response, '>', 'f')

instead of:

f = struct.unpack('>f', struct.pack('>H', high_byte) + struct.pack('>H', low_byte)[0]
coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.06%) to 96.362% when pulling 1f42cbc2a8b398c396ce4045596f511e689711e1 on Simonefardella:master into 0560a42308003f4072d988f28042b8d55b694ad4 on AdvancedClimateSystems:master.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.06%) to 96.362% when pulling 1f42cbc2a8b398c396ce4045596f511e689711e1 on Simonefardella:master into 0560a42308003f4072d988f28042b8d55b694ad4 on AdvancedClimateSystems:master.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.06%) to 96.362% when pulling 1f42cbc2a8b398c396ce4045596f511e689711e1 on Simonefardella:master into 0560a42308003f4072d988f28042b8d55b694ad4 on AdvancedClimateSystems:master.

OrangeTux commented 5 years ago

Thanks for the PR! You have added tests and docstrings: i think it's a clean PR.

But I'm bit in doubt about the PR. I think your PR is trying to solve a problem that people have: packing and unpack of values bigger than 16-bits is a bit hard. Although most packing/unpacking can done sometimes even with 1 line, it is not very readable. So I was against it to add those utility functions in [this comment], I changed my mind. I think pack/unpack helpers would be useful in uModbus.

But I think this PR doesn't solve the problem correctly, the API you propose is still not so easy to use for people unfamiliar with the struct package. You still need to know how the the symbols for endiannes, floats, longs etc. So therefore I'm rejecting the PR for now.

But this PR made me realize that easy-to-read pack/unpack should be added to uModbus. Thanks for that.

If you've an idea for a concise, but very clear API for some pack/unpack helpers: feel free to open this PR. I currently can't come up with a nice API, but I'll put some thought into it this weekend.

rtu-dataframe commented 5 years ago

No problem Man, my solution it's based on the Modbus-tk philosophy, in any case, this was my concept: it's not easy to pack and unpack the values bigger than 16 bit and you need to know very well the indianess and the data type.

In any case, feel free to take the idea or the concept from my PR.