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

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

Closed

Conversation

rtu-dataframe
Copy link

@rtu-dataframe rtu-dataframe commented Jan 14, 2019

@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
Copy link

Coverage Status

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

2 similar comments
@coveralls
Copy link

Coverage Status

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

@coveralls
Copy link

Coverage Status

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

@OrangeTux OrangeTux assigned OrangeTux and unassigned OrangeTux Jan 16, 2019
@OrangeTux
Copy link
Collaborator

OrangeTux commented Jan 16, 2019

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.

@OrangeTux OrangeTux closed this Jan 16, 2019
@rtu-dataframe
Copy link
Author

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.

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.

3 participants