stlehmann / pyads

Python wrapper for TwinCAT ADS
MIT License
264 stars 96 forks source link

Added feature for processing nested structures #384

Closed msfur closed 2 months ago

msfur commented 6 months ago

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 commented 6 months ago

Pull Request Test Coverage Report for Build 10633564917

Details


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

💛 - Coveralls
chrisbeardy commented 6 months ago

@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 commented 6 months ago

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

keenanjohnson commented 4 months ago

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

kryskool commented 3 months ago

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)

msfur commented 2 months ago

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

kryskool commented 2 months ago

@chrisbeardy this PR is ok for merging

chrisbeardy commented 2 months ago

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

chrisbeardy commented 2 months ago

@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.

keenanjohnson commented 2 months ago

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 commented 2 months ago

@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 commented 1 month ago

@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

keenanjohnson commented 3 weeks ago

Will there be a new published release of pyads that includes this soon? Itching to use it officially.