dmroeder / pylogix

Read/Write data from Allen Bradley Compact/Control Logix PLC's
Apache License 2.0
598 stars 182 forks source link

Endianness issue in Jython #132

Closed JordanCClark closed 4 years ago

JordanCClark commented 4 years ago

Versions

Since Jython uses Java to emulate python libraries, the endianness of native pack/unpack is reversed.

Setting self.CIPTypes tuples to force little endian fixes the issue, and still seems to work under Python 3.8

        self.CIPTypes = {0: (0, "UNKNOWN", '?'),
                         160: (88, "STRUCT", '<B'),
                         193: (1, "BOOL", '?'),
                         194: (1, "SINT", '<b'),
                         195: (2, "INT", '<h'),
                         196: (4, "DINT", '<i'),
                         197: (8, "LINT", '<q'),
                         198: (1, "USINT", '<B'),
                         199: (2, "UINT", '<H'),
                         200: (4, "UDINT", '<I'),
                         201: (8, "LWORD", '<Q'),
                         202: (4, "REAL", '<f'),
                         203: (8, "LREAL", '<d'),
                         211: (4, "DWORD", '<I'),
                         218: (1, "STRING", '<B')}
TheFern2 commented 4 years ago

@JordanCClark Thanks for opening the issue, we'll investigate.

@dmroeder I thought we had a good share of ignition users, would seem odd that no one would report it until now.

JordanCClark commented 4 years ago

I helped do some testing on the early versions. Thought I'd take a look again, now that I have a bit of extra time on my hands.

And to be fair, I'd say most Ignition users are reading/writing via OPC functions. Ignition just happens to be my most convenient Jython installation. Even then I had to adjust the imports since Ignition doesn't always handle relative imports very well.

Let me spin up a VM with Jython by itself. I'll let you know what I find.

JordanCClark commented 4 years ago

Confirmed with

Java uses big-endian across all platforms, so that it doesn't care (assuming Java-only applications) what the native endianness is.

https://howtodoinjava.com/java/basics/little-endian-and-big-endian-in-java/

TheFern2 commented 4 years ago

Thanks for the details. At first glance it seems like the issue is jython related.

@dmroeder We haven't explicitly said we support Jython. How do you feel about this patch, assuming it doesn't break any current python unit testing?

dmroeder commented 4 years ago

@kodaman2 I did some quick testing and it doesn't seem to have any negative impact so if it helps the great @JordanCClark, I'm on board.

TheFern2 commented 4 years ago

I'll push patch right after I push small change to PylogixTests.py it broke after structuring the tag and response classes to their own file.

dmroeder commented 4 years ago

Shoot, that was my bad (obviously)

TheFern2 commented 4 years ago

Shoot, that was my bad (obviously)

No worries, you sent me the new structure too, and I missed it.

Patch pushed in #134 @JordanCClark please install from head until there is a release available. Let us know how it went.

pip install git+https://github.com/dmroeder/pylogix
JordanCClark commented 4 years ago

Looks good, gentlemen. Many thanks for the support!