Skip to content
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.

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

Added feature for processing nested structures #384

Merged

Conversation

msfur
Copy link
Contributor

@msfur msfur commented May 7, 2024

Enables processing for nested structures, as mentioned in and fixes #308 and on Stackoverflow. Takes into account nested structures in the following form:

    substructure_def = (
         ("iVar", pyads.PLCTYPE_INT, 1),
         ("sVar", pyads.PLCTYPE_STRING, 1)
     )

    structure_def = (
         ("iVar1", pyads.PLCTYPE_INT, 1),
         ("structVar", substructure_def, 2)
     )

or

    structure_def = (
        ("iVar1", pyads.PLCTYPE_INT, 1),
        ("structVar",
            (
                ("iVar", pyads.PLCTYPE_INT, 1),
                ("sVar", pyads.PLCTYPE_STRING, 1)
            ), 2
        )
    )

Contains adaptations to size_of_structure(), dict_from_bytes() and bytes_from_dict() and the associated tests.

@coveralls
Copy link

coveralls commented May 7, 2024

Pull Request Test Coverage Report for Build 10633564917

Details

  • 9 of 9 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 94.951%

Totals Coverage Status
Change from base Build 10631185064: 0.03%
Covered Lines: 1730
Relevant Lines: 1822

💛 - Coveralls

@chrisbeardy
Copy link
Collaborator

@msfur this is awesome, looks great as a solution for the nested structs issue and thanks for adding the tests in too! If possible, could you please add a documentation example at the end of this section too? https://pyads.readthedocs.io/en/latest/documentation/connection.html#structures-with-multiple-datatypes

I will then merge.

@msfur
Copy link
Contributor Author

msfur commented May 16, 2024

@chrisbeardy I have made the desired change to the documentation and added an example.

@keenanjohnson
Copy link

This looks great! Any way I can support getting this merged?

@kryskool
Copy link
Contributor

Hi @msfur
I validate this merge request with my tests (pytest) for my hospital cabinet, and nested structure works perfectly

This PR is ok for me (must be rebase)

Copy link
Contributor

@kryskool kryskool left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nested Structure works (test also with TIMESTRUCT)

@msfur msfur force-pushed the feature/support_for_nested_structures branch from f0dbc8a to d3e8bbd Compare August 30, 2024 12:57
@msfur
Copy link
Contributor Author

msfur commented Aug 30, 2024

Hi @kryskool,
thanks for the approval. I have rebased the branch as requested.

@kryskool
Copy link
Contributor

@chrisbeardy this PR is ok for merging

added section 3.5.0 (unreleased to changelog)
@chrisbeardy chrisbeardy merged commit 7a27561 into stlehmann:master Aug 30, 2024
5 checks passed
@chrisbeardy
Copy link
Collaborator

thanks all for this, will release new version soon, couple of other PRs to merge

@chrisbeardy
Copy link
Collaborator

@msfur thanks for the contribution, this is a great edition to pyads. Since you've wetted your palette now with this PR, theres a open issue related to structs in #289 if you wanted to have a look and this is relevant to helping with your workflow.

@msfur msfur deleted the feature/support_for_nested_structures branch August 30, 2024 13:40
@msfur msfur restored the feature/support_for_nested_structures branch August 30, 2024 13:47
@msfur msfur deleted the feature/support_for_nested_structures branch August 30, 2024 13:48
@keenanjohnson
Copy link

Great work getting this merged in all! I know I'm just a random person on the internet, but working together like this on software that most people will never hear about is truly useful for hummanity and I appreciate you. - Some random user of pyads

@msfur
Copy link
Contributor Author

msfur commented Sep 20, 2024

@msfur thanks for the contribution, this is a great edition to pyads. Since you've wetted your palette now with this PR, theres a open issue related to structs in #289 if you wanted to have a look and this is relevant to helping with your workflow.

I'm going to check it out and see if there's anything I can contribute.

@tom-ils
Copy link

tom-ils commented Sep 30, 2024

@msfur thanks for this great feature- I have been working with your branch of pyads for a few months now so great to see it merged into master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: read a struct of struct
6 participants