Problems with strings using read and readwrite

[ChristianHemann]

Hello,

in a recent project we are using pyads to communicate with a plc, especially by using methods which we are accessing via readwrite. Also we need to read some long strings (length 10.000).
Both was working in pyads 3.3.4, but when updating to pyads 3.3.9 it starts failing. It seems as the problem is caused by the introduction of the helper function type_is_string() in pyads_ex

First Problem: Reading long strings

In pyads 3.3.4 we were specifying the length of the string and pyads was returning the read bytes that we had to parse in the way we need them.
Our code was looking like this:

value: bytes = plc_connection.read_by_name(
    "MAIN.VariableName",
    plc_datatype=pyads.PLCTYPE_STRING * (10000 + 1),
)

With pyads 3.3.9, pyads is using type_is_string(), which does not just check for data_type == PLCTYPE_STRING, but also checks for "PyCArrayType". If it finds a string, which is now true for the datatype above, it is not using my string type with a custom length, but using the default string length of 1024 instead (STRING_BUFFER * PLCTYPE_STRING)

My suggestion to fix this would be to change
type_is_string(read_data_type) into
type_is_string(read_data_type) and type(read_data_type).__name__ != "PyCArrayType"
This would need to be done in adsSyncReadReqEx2() and adsSyncReadWriteReqEx2(). Maybe in other functions as well, which I have not tested.

One change that I like in pyads 3.3.9 ist, that read_by_name is now returning a string, instead of a bytes-object.

Second Problem: Calling Methods with a single string parameter

We have a method taking a single parameter of type string with a length of 255. It returns a struct, but that seems not to be the problem.
When calling this functions via read_write, ADS is raising an Error, that the size is not correct. Our code looks similar to this:

handle_path = f"MAIN.MyObject#FunctionName"
handle = plc_connection.get_handle(handle_path)

parameter_type=pyads.PLCTYPE_STRING * (255 + 1)
parameters_value = "value".encode("ASCII")

return_value = plc_connection.read_write(
    pyads.constants.ADSIGRP_SYM_VALBYHND,
    handle,
    plc_read_datatype=return_type,
    plc_write_datatype=parameter_type,
    value=parameter_value,
)

In pyads 3.3.4 the length of the string was determined by adsSyncReadWriteReqEx2 as:

write_data = write_data_type(*value)
write_length = ctypes.c_ulong(ctypes.sizeof(write_data))

In pyads 3.3.9 the length of the string is determined as write_length = len(value) + 1

In the old implementation the size were the correct 256 bytes. In the new version it is just 7 bytes.
I am not sure if the length of 7 bytes would be correct in some scenarios. If not I would suggest:

if type(write_data_type).__name__ != "PyCArrayType":  # PLCTYPE_STRING without length
    write_length = STRING_BUFFER + 1  # not tested if it works
else:
    write_length = ctypes.sizeof(write_data_type) + 1

One thing to notice is, that with pyads 3.3.9, I do not have to encode the string as bytes manually. I think that's a good point.

Note that I have not tested all of the code above. Maybe there are some small bugs introduced while writing this.

Could you please have a look into this topic and create a newer version of pyads if necessary?

best regards,
Christian

[chrisbeardy]

Hi Christian, great observations and sorry that these changes have caused you issues. I'm sure that at some point this will get solved, unfortunately I can't guarantee quick results as this project is maintained by volunteers only and progress seems to have slowed down recently. You seemed to have already looked into this topic and found some potential solutions, please feel free to make these suggested edits and do the testing to make sure that any solutions don't cause other issues then submit a PR, it would be great to get your help.

[chrisbeardy]

It looks like for a workaround for the first one you can use:

value = bytearray(plc_connection.read_by_name(
    "MAIN.VariableName",
    plc_datatype=pyads.PLCTYPE_BYTE * (10000 + 1),
)).decode().strip("\00")

[chrisbeardy]

this workaround using PLCTYPE_BYTE may also work for the methiod call too

[ChristianHemann]

Hi Chris,
thanks for the reply. I think I will try the workaround using PLCTYPE_BYTES later on.
If I get the time I might look deeper into it and create a PR, but I cannot guarantee for it.

EDIT:
The workaround is working, even for method calls.

best regards
Christian

[jodle001]

It looks like for a workaround for the first one you can use:

value = bytearray(plc_connection.read_by_name(
    "MAIN.VariableName",
    plc_datatype=pyads.PLCTYPE_BYTE * (10000 + 1),
)).decode().strip("\00")

Thank you this worked for my project here where I wrapped your library in a ros2 package. https://github.com/jodle001/ros2_pyads

[RobertoRoos]

I see this is more or less the same issue as #420
This would be resolved by PR #419