read_by_list does not read arrays

chrisbeardy commented 3 years ago

Not sure if by design (or a known limition) or if this is a bug.

When reading an array using read_by_name (either with or without passing in datatype), you recieve back a list with all array elements. However when reading it using read_list_by_name it returns only the first element.

kitcheung1480 commented 3 years ago

I think this is a bug. As I dig into the code, I find out that they didn't deal with plc array types. Here is the quick fix: pyads.pyads_ex line 921-925 update with

if data_symbols[data_name].size > ctypes.sizeof(ads_type_to_ctype[data_symbols[data_name].dataType]):
    value = struct.unpack_from(
        "<" + DATATYPE_MAP[ads_type_to_ctype[data_symbols[data_name].dataType]][-1] * (data_symbols[data_name].size // ctypes.sizeof(ads_type_to_ctype[data_symbols[data_name].dataType])),
        sum_response,
        offset=data_start + offset,
    )
else:
    value = struct.unpack_from(
        DATATYPE_MAP[ads_type_to_ctype[data_symbols[data_name].dataType]],
        sum_response,
        offset=data_start + offset,
    )[0]

This also work for write_list_by_name. pyads.pyads_ex line 983-988 update with

if data_symbols[data_name].size > ctypes.sizeof(ads_type_to_ctype[data_symbols[data_name].dataType]):
    struct.pack_into(
        "<" + DATATYPE_MAP[ads_type_to_ctype[data_symbols[data_name].dataType]][-1] * (data_symbols[data_name].size // ctypes.sizeof(ads_type_to_ctype[data_symbols[data_name].dataType])),
        buf,
        offset,
        *value,
    )
else:
    struct.pack_into(
        DATATYPE_MAP[ads_type_to_ctype[data_symbols[data_name].dataType]],
        buf,
        offset,
        value,
    )

stlehmann commented 2 years ago

@kitcheung1480, @chrisbeardy thanks for looking into this. Would you mind creating a PR for this fix and implement a test that covers this issue?

HectorBailey commented 1 year ago

Hi @stlehmann ,

I've just encountered this issue in a project I'm currently working on and have implemented the code written above by "kitcheung1480". I've also implemented support for string and wstring arrays, which i have tested with TC3 - and it all works well.

The only problem I have is that I'm struggling to understand how to use the test server. I've managed to write a test that passes for an array of wstrings, but can't seem to get it working with the others. I think it's down to the "ads_type" parameter when adding an array to the test server.

I would like to make a pull request so others can also benefit from this bug fix, but I guess we kinda need the tests working before we do that. Any help with this would be much appreciated.

Here is my forked repo that I am making the changes on - so you can see what I've done: https://github.com/ChurchillSD/pyads-read-list-by-name-array-fix

chrisbeardy commented 1 year ago

it may be best to split up the PR into two, one to implement the fix and one to support wstring arrays (if the two are not intrinsically linked). As I assume the tests for the fix would be easier to implement. I suspect the changes for the string array support may be more involved?

HectorBailey commented 1 year ago

Just got around to sorting it out, the pull request has been made with the all the changes mentioned.

stlehmann / pyads

read_by_list does not read arrays #252