Problem with some setters

[core-engineering]

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]

@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]

@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]

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.

[gijzelaerr]

@core-engineering i you are still up to it i dont mind merging a PR where the objects are returned, since this would not break any backwards compatiblity.

[gijzelaerr]

Hi! A fix for this has been submitted in PR #585. The setters that were returning None (set_fstring, set_string, set_dword, set_dint, set_udint) now consistently return the modified bytearray, matching the behavior of the other setters.

Could you please take a look and confirm whether this resolves your issue? Any feedback would be appreciated!