gijzelaerr / python-snap7

A Python wrapper for the snap7 PLC communication library
http://python-snap7.readthedocs.org/
MIT License
670 stars 245 forks source link

Problem with some setters #553

Open core-engineering opened 4 days ago

core-engineering commented 4 days ago

Hi,

First, I would like to say that this python library is quite usefull to me ! Thanks a lot for the contributors and the maintainer. The original snap7 library is also awesome !

During one dev, I couldn't use the util.set_dint and the util.set_udint methods properly.

After code check, it appears that the return statement is missing in some util.set methods.

Original code :

def set_dint(bytearray_: bytearray, byte_index: int, dint: int) -> None:

    dint = int(dint)
    _bytes = struct.unpack("4B", struct.pack(">i", dint))
    for i, b in enumerate(_bytes):
        bytearray_[byte_index + i] = b

Working code :

def set_dint(bytearray_: bytearray, byte_index: int, dint: int) -> bytearray:

    dint = int(dint)
    _bytes = struct.unpack("4B", struct.pack(">i", dint))
    for i, b in enumerate(_bytes):
        bytearray_[byte_index + i] = b
    return bytearray_

If needed, I can propose a pull request !

Camille

lupaulus commented 4 days ago

@core-engineering Hello Camille,

Thanks for the proposal but you don't to return the bytearray, the original bytearray has been updated. See this example from the docs :

>>> data = bytearray(4)
>>> set_udint(data, 0, 4294967295)
>>> data
>>> bytearray(b'\\xff\\xff\\xff\\xff')
core-engineering commented 3 days ago

@lupaulus Hi Lucas,

I agree with you but this behaviour is not consistent with all the setters. For example, the set_int method is the following :

def set_int(bytearray_: bytearray, byte_index: int, _int: int) -> bytearray:
    _int = int(_int)
    _bytes = struct.unpack("2B", struct.pack(">h", _int))
    bytearray_[byte_index : byte_index + 2] = _bytes
    return bytearray_

Just to be a little more specific, please find my code to send to the PLC a DTL type :

current_dateTime = datetime.now()
clientInstance.db_write(
    db_number = 2,
    start = 0,    
    data = (
        util.set_uint(bytearray(2), 0, current_dateTime.year)
        + util.set_usint(bytearray(1), 0, current_dateTime.month)
        + util.set_usint(bytearray(1), 0, current_dateTime.day)
        + util.set_usint(bytearray(1), 0, current_dateTime.isoweekday())
        + util.set_usint(bytearray(1), 0, current_dateTime.hour)
        + util.set_usint(bytearray(1), 0, current_dateTime.minute)
        + util.set_usint(bytearray(1), 0, current_dateTime.second)
        + util.set_udint(bytearray(4), 0, current_dateTime.microsecond*1000)
    )
)

Everything is ok with the util.set_uint and util.set_usint as these methods return effectivelly the bytearray but this is not the case for the util.set_udint.

Is it possible to have consistancy accross all the setters ?

Camille

gijzelaerr commented 3 days ago

i think it could be more pythonic to eliminate the need to supply an empty bytearrays all together, but then we need to make sure nobody needs to set a partial bytearray.