stlehmann / pyads

Python wrapper for TwinCAT ADS
MIT License
254 stars 94 forks source link

Add WSTRING support #274

Closed stlehmann closed 2 years ago

stlehmann commented 2 years ago

Addresses #246

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 1467908502


Changes Missing Coverage Covered Lines Changed/Added Lines %
pyads/pyads_ex.py 42 45 93.33%
<!-- Total: 43 46 93.48% -->
Totals Coverage Status
Change from base Build 1439873570: 0.2%
Covered Lines: 1663
Relevant Lines: 1766

💛 - Coveralls
stlehmann commented 2 years ago

Looks like there needs to be some more effort here. Strings are handled special in read and write functions. So that needs to be applied to WSTRINGS as well.

stlehmann commented 2 years ago

Now we also need some testing.

stlehmann commented 2 years ago

I just made a Wireshark dump to see if the WSTRING is null-terminated with 1 or 2 Bytes:

grafik

Clearly it is two bytes null-termination. So the testserver has to resemble this behaviour.

kryskool commented 2 years ago

Hi @stlehmann

i have an equipement with WSTRING, i try next week this PR

Regards,

stlehmann commented 2 years ago

I just have a use-case where I use WSTRINGs and discovered that adsSumRead and adsSumWrite also need specific WSTRING support

stlehmann commented 2 years ago

Tested the implementation on my PC but couldn't get the test-cases pass, yet. Somehow the encoding is not right. Suggestions are welcome 😊

stlehmann commented 2 years ago

@kryskool did you already do some testing on your setup with this branch?

stlehmann commented 2 years ago

Todo: handle WSTRINGS right in adsSyncReadWriteReqEx2 and in adsSyncWriteControlReqEx