Skip to content

Add WSTRING support for structures #293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
stlehmann merged 21 commits into from
Dec 14, 2021
Merged

Add WSTRING support for structures #293

stlehmann merged 21 commits into from
Dec 14, 2021

Conversation

@stlehmann
Copy link
Owner

@stlehmann stlehmann commented Nov 29, 2021 •
edited
Loading

Targets #291

@stlehmann stlehmann changed the title Add WSTRING support for structures (#291) Add WSTRING support for structures ( Nov 29, 2021
@stlehmann stlehmann changed the title Add WSTRING support for structures ( Add WSTRING support for structures Nov 29, 2021
@coveralls
Copy link

coveralls commented Nov 29, 2021 •
edited
Loading

Pull Request Test Coverage Report for Build 1576787639

  • 36 of 36 (100.0%) changed or added relevant lines in 3 files are covered.
  • 6 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.2%) to 94.255%
Files with Coverage Reduction New Missed Lines %
pyads/pyads_ex.py 1 90.37%
pyads/testserver/testserver.py 5 90.76%
Totals Coverage Status
Change from base Build 1519582091: -0.2%
Covered Lines: 1690
Relevant Lines: 1793
💛 - Coveralls

@stlehmann stlehmann force-pushed the add-wstring-support-for-structures branch from a3955e1 to dc7f7d4 Compare December 9, 2021 12:28
@stlehmann
Add helper function to find null-terminator in WSTRING
b21a227
@stlehmann
Add test for find_wstring_null_terminator function
43c7ca0
@stlehmann
Use find_wstring_null_terminator function in adsSumRead
4f84d83
@stlehmann
Add WSTRING support for size_of_structure
60c47df
@stlehmann
Fix size_of_structure.
fd845eb 
Broke it in the commit before by modifying the conditions.
@stlehmann
Add WSTRING support for bytes_from_dict
8e70db6
@stlehmann
Fix bytes_from_dict
3e83779 
Array size was not calculated correctly
@stlehmann
Use length of bytearray
fe198e5
@stlehmann
Improve performance on adding remaining bytes
66932bb 
This may not be necessary but replacing the for loop with the extend statement speeds speeds it up by factor 5
@stlehmann
Fix size_of_structure for multiple fields
5f9ac8d
@stlehmann
Add test for read/write WSTRING struct
c7e80a3
@stlehmann
Add test for read/write WSTRING array struct
16edaad
@stlehmann
Add Changelog entry
4626b1c
@stlehmann stlehmann marked this pull request as ready for review December 10, 2021 09:32
@stlehmann
Copy link
Owner Author

stlehmann commented Dec 10, 2021

This adds support for WSTRINGs in structures, both as fields and as arrays.
@chrisbeardy would you mind taking a look at this PR and giving a short review as this involves some changes on the codebase written by you.

chrisbeardy
chrisbeardy reviewed Dec 12, 2021
View reviewed changes
chrisbeardy
chrisbeardy reviewed Dec 12, 2021
View reviewed changes
chrisbeardy
chrisbeardy reviewed Dec 12, 2021
View reviewed changes
@stlehmann
Fix str_len
f9f7ffe 
- str_len now is always the number of characters without null-terminator
- n_bytes is the number of bytes and differs for STRING or WSTRING
@stlehmann
Add tests for size_of_structure for WSTRING
7e57d73
@stlehmann stlehmann force-pushed the add-wstring-support-for-structures branch from 6051aa3 to 7e57d73 Compare December 13, 2021 12:54
@stlehmann stlehmann merged commit fe00679 into master Dec 14, 2021
@stlehmann stlehmann deleted the add-wstring-support-for-structures branch December 14, 2021 08:09
@stlehmann stlehmann mentioned this pull request Dec 15, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chrisbeardy chrisbeardy chrisbeardy left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@stlehmann @coveralls @chrisbeardy