search

ljean / modbus-tk

Create Modbus app easily with Python
Other
571 stars 212 forks source link

Python3 Compatibility #49

Closed iwoloschin closed 5 years ago

iwoloschin commented 9 years ago

I'm not sure if this is something you're looking for, but I found a minor issue with Python3 compatibility in modbus_tcp.py. It looks like you're in the middle of adding Python3 support, but if not feel free to close/ignore this. Also, I've run 2to3 on a local copy of modbus_tk in addition to the fix outlined below (2to3 didn't seem to catch this one problem).

/usr/local/lib/python3.5/site-packages/modbus_tk-0.5.0-py3.5.egg/modbus_tk/modbus_tcp.py in _recv(self, expected_length) 215 rcv_byte = self._sock.recv(1) 216 if rcv_byte: --> 217 response += rcv_byte 218 if len(response) == 6: 219 to_be_recv_length = struct.unpack(">HHH", response)[2]

TypeError: Can't convert 'bytes' object to str implicitly

This is because the variable response is initialized like so:

Line 212: response = ""

This works fine in Python2, but not Python3. The fix is simple though, initialize response like this:

Line 212: response = "".encode()

or:

Line 212: response = b""

Though I think the first is a little clearer and more obvious. Probably plenty of other ways to fix this as well, but this is simple and makes my project work well (so far!) with Python3 and modbus_tk!

zerox1212 commented 9 years ago

I'm about to use the RTU portion of modbus-tk in Python 3.4. Do you want to make a fork for your python 3 implementation? Otherwise I can fork it for Python 3+ support. I don't know a lot about Python so I don't know the proper way to add support for Python 3 (setup.py creation, etc.), but I'll still try. :)

zerox1212 commented 9 years ago

Can someone explain the use of this from modbus_rtu.py:

(self._response_address, ) = struct.unpack(">B", response[0])

Why unpack a single response array member that's already a byte? Python will treat this as an INT anyways. Or was this not the case in Python 2? (I've only use Python 3.4)

ljean commented 9 years ago

Hello,

The current version should be prepare for Python 3.4 but may be everything doesn't work perfectly... so feel free to fork and to send a pull request :-) It would be nice to keep python 2 and Python 3 compatibility for the same codebase so I recommend to use the six module.

I think that the struct.unpack(">B", response[0]) makes sense for Python 2. It should not be removed if we want to keep Python 2 compatibility.

Best luc

2015-10-27 21:47 GMT+01:00 zerox1212 notifications@github.com:

Can someone explain the use of this from modbus_rtu.py: ''' (self._response_address, ) = response[0] # struct.unpack(">B", response[0]) '''

Why unpack a single response that's already a byte? Python will treat this as an INT anyways. Or was this not the case in Python 2? (I've only use Python 3.4)

— Reply to this email directly or view it on GitHub https://github.com/ljean/modbus-tk/issues/49#issuecomment-151638822.

zerox1212 commented 9 years ago

I have modbus-tk running in Python 3.4 using ModbusRTU. I just ran the script 2to3.py on the code then fixed a few type errors (because string and byte arrays are different in Python 3). Unfortunately so far I have not found a way to fix the struct.unpack code in Python 3 that would also be OK for 2.

For now I just did this:

self._response_address = response[0]

I have forked the code and I will try to make it Python 2 and 3 compatible.

ljean commented 8 years ago

I've made a commit for Python3 support. I would like to know if it runs ok for you. It is still under development.

rEegLer commented 8 years ago

Hi Luc,

Any update on Python 3 support? My application is creating a RPi CAN bus to Modbus gateway and python-can only works with python 3.

ljean commented 8 years ago

Hello,

This is still under test but the current master should work with python 3. It seems to work fine but there is a timing issue pointed by Yuri in another thread discussion. I didn't have time to look at this yet.

Best luc

2016-02-04 15:51 GMT+01:00 rEegLer notifications@github.com:

Hi Luc,

Any update on Python 3 support? My application is creating a RPi CAN bus to Modbus gateway and python-can only works with python 3.

— Reply to this email directly or view it on GitHub https://github.com/ljean/modbus-tk/issues/49#issuecomment-179880347.

drvlas commented 8 years ago

Hi all,

I used modbus_tk (RTU) for a while with python2.7. Now I migrate to python3. Luc gave me a version (called modbus_new) to try. It runs ok, basically. The only problem I've met is a timeout issue. Here are the details.

1) modbus Master sends a request to a Slave and the Slave responds instantly (all transactions are less then 40 ms in duration - oscilloscope says). So far so good... 2) But Modbus stack returns the result in 1502 ms after the begin of request.

I have a screenshot of debug traffic but I don't see how can I attach it here. So here is a link to the image.

The screenshot shows exactly 1502 ms: e.g. 2016-01-06 11:13:5.924 - 2016-01-06 11:13:4.422 Where does it waste all this time? I have found that such a patch helps:

/modbus_new/modbus_tk/modbus_rtu.py class RtuMaster method _send():

time.sleep(self.get_timeout())

Now it works. Regards, Yury

zerox1212 commented 8 years ago

It's been a while since I took a look at this as my hack is still working.

I reviewed your code changes and it looks good to me. It would be nice to remove dependency on the six module though. From what I saw you could probably just add this to utils.py:

PY2 = sys.version_info[0] == 2
PY3 = sys.version_info[0] == 3

Then you can replace six.PY2 with utils.PY2 and drop the dependency. However maybe you used some other functions of six that I didn't see....

I will grab your Python 3 version and test it shortly and let you know what I find.

mindergrinder commented 8 years ago

Any response to drvlas's post? Is mentioned sleep on line 124 in modbus_rtu.py there for a reason? Because it slows down things a lot. Are you really suppose to control timeout in such way?

ljean commented 8 years ago

Hello,

Sorry, I didn't have time to check it yet. I don't think this timeout should be necessary. Maybe it is due to synchronization issue.

Please let me some time to investigate more.

Best luc

2016-03-15 15:28 GMT+01:00 mindergrinder notifications@github.com:

Any response to drvlas's post? Is mentioned sleep on line 124 in modbus_rtu.py there for a reason? Because it slows down things a lot. Are you really suppose to control timeout in such way?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/ljean/modbus-tk/issues/49#issuecomment-196845198