Transaction ID overflow

greg0pearce commented 7 years ago

Firstly, fantastic job Auke.

When I run some dummy code

#!/usr/bin/env python
# scripts/examples/simple_tcp_client.py
import socket
import sys
from umodbus import conf
from umodbus.client import tcp

conf.SIGNED_VALUES = True
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
sock.connect(('10.10.1.8', 502))

i=0
while True:
    try:
        i+=1
        message = tcp.read_discrete_inputs(slave_id=1, starting_address=0,quantity=1)
        response = tcp.send_message(message, sock)
    except:
        print("Unexpected error:", sys.exc_info())
        print(i)

I receive error

File "C:\Python34\lib\site-packages\umodbus\client\tcp.py", line 106, in _create_mbap_header return struct.pack('>HHHB', transaction_id, 0, length, slave_id) struct.error: 'H' format requires 0 <= number <= 65535

I think uModbus/umodbus/client/tcp.py Line 103 be (2^16)-1=65535 to fit in 2 bytes

#65536 = 2**16 aka maximum number that fits in 2 bytes.
transaction_id = randint(0, **65536**)
length = len(pdu) + 1

Best Regards, Greg Pearce

OrangeTux commented 7 years ago

@greg0pearce Thanks for reporting, clever finding!

On your profile I see that you're a new user. Although the solution is trivial, are you interested in fixing this bug yourself to take your first steps in open source development?

If so, fork the repository and checkout the develop branch. Write your fix. Push those commits on the develop branch of your fork and create a pull request on the develop branch of this project.

It's no problem when you don't want to fix it yourself. In that case i'll fix it.

greg0pearce commented 7 years ago

Thanks Auke I would like that opportunity. I should be able to do that tonight.

I am new to programming in general, I’m currently reading a book on python, but I find it more beneficial to read code done by much better programmers such as yourself!

Thanks again and best regards from Australia.

Greg Pearce

From: Auke Willem Oosterhoff [mailto:notifications@github.com] Sent: Monday, 26 September 2016 8:28 PM To: AdvancedClimateSystems/uModbus uModbus@noreply.github.com Cc: Greg Pearce greg.pearce.9876@gmail.com; Mention mention@noreply.github.com Subject: Re: [AdvancedClimateSystems/uModbus] Transaction ID overflow (#43)

@greg0pearce https://github.com/greg0pearce Thanks for reporting, clever finding!

On your profile I see that you're a new user. Although the solution is trivial, are you interested in fixing this bug yourself to take your first steps in open source development?

If so, fork the repository and checkout the develop branch. Write your fix. Push those commits on the develop branch of your fork and create a pull request on the develop branch of this project.

It's no problem when you don't want to fix it yourself. In that case i'll fix it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/AdvancedClimateSystems/uModbus/issues/43#issuecomment-249555873 , or mute the thread https://github.com/notifications/unsubscribe-auth/ATa8mO3XsomgOUcsJlWmm9u7A-aBtDQkks5qt7o_gaJpZM4KGCLG . https://github.com/notifications/beacon/ATa8mFULS_xmBie8FpfKu8A-f8o-Wy1xks5qt7o_gaJpZM4KGCLG.gif

OrangeTux commented 7 years ago

Fixed in 0.7.2

AdvancedClimateSystems / uModbus

Transaction ID overflow #43