ruscito / pycomm

pycomm is a package that includes a collection of modules used to communicate with PLCs
MIT License
142 stars 85 forks source link

os.getpid() doesn't fit into word, problem with sequence number #73

Open jk987 opened 2 years ago

jk987 commented 2 years ago
File "C:\\Python27\\lib\\site-packages\\pycomm\\ab_comm\\slc.py", line 242, in read_tag
    self._last_sequence = pack_uint(Base._get_sequence())
File "C:\\Python27\\lib\\site-packages\\pycomm\\cip\\cip_base.py", line 63, in pack_uint
    return struct.pack(\'<H\', n)
error: ushort format requires 0 <= number <= USHRT_MAX

Method Base._get_sequence() uses os.getpid() but this can be (and is) higher than 65535.

Btw. I don't like this logic in Base._get_sequence() much:

        if Base._sequence < 65535:
            Base._sequence += 1
        else:
            Base._sequence = getpid()

I would understand that if sequence number overflows then it should start from zero again, shouldn't it? So, in Base.__init__() I recommend to put:

        if Base._sequence == 0:
            Base._sequence = getpid() & 0xFFFF
        else:
            Base._sequence = Base._get_sequence()

and in Base._get_sequence():

        if Base._sequence < 65535:
            Base._sequence += 1
        else:
            Base._sequence = 0

Could be written also like Base._sequence = (Base._sequence + 1) & 0xFFFF which would handle even negative numbers if such thing can happen.