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

Return data gets jumbled up on a multi write when the tags don't exist. #224

Closed
Destination2Unknown opened this issue May 8, 2023 · 13 comments
Labels

Comments

@Destination2Unknown
Copy link

Code

from pylogix import PLC

with PLC() as comm:
    comm.IPAddress = "192.168.1.9"

    write_data = [("tag1", 100), ("tag2", 6.45), ("tag3", True)]

    # write the values
    ret = comm.Write(write_data)

    # print the status of the writes
    for r in ret:
        print(r.TagName, r.Status)

Prints:

tag2 Unknown error timed out
6.45 Unknown error timed out

Versions

  • pylogix: 0.8.11
  • python: 3.11
  • OS: Windows
@dmroeder dmroeder added the bug label May 8, 2023
@dmroeder
Copy link
Owner

dmroeder commented May 8, 2023

I have a fix for this.

@Destination2Unknown
Copy link
Author

@dmroeder Sweet!

@Destination2Unknown
Copy link
Author

I think this was the issue:

pylogix/pylogix/eip.py

Lines 435 to 436 in 95d6bc9

if not conn[0]:
return [Response(t, None, conn[1]) for t in tags[1]]

It should be something like:

if not conn[0]:
    return [Response(t[0], None, conn[1]) for t in tags]

dmroeder added a commit that referenced this issue May 11, 2023
This reverts a previous commit to deal with data types that were assinged
a value of 0.  This will have to be investigated further.

Fixes #224
@dmroeder
Copy link
Owner

You mind testing the bugfix/multi-write branch when you get time? It works for me, though things weren't quite failing in the same way for me.

@Destination2Unknown
Copy link
Author

Pretty much sorted, only case where it didn't work was where the tags don't exist and there is no connection to the PLC (i.e. if the IP Address is incorrect):

from pylogix import PLC

with PLC() as comm:
    comm.IPAddress = "192.168.133.199"

    write_data = [("tag1", 100), ("tag2", 6.45), ("tag3", True)]

    # write the values
    ret = comm.Write(write_data)

    # print the status of the writes
    for r in ret:
        print(r.TagName, r.Status)

prints:

tag2 Unknown error timed out
6.45 Unknown error timed out

#224 (comment)

@Destination2Unknown
Copy link
Author

This fairly niche example isn't quite right either...

write_data = [("myDint", 100), ("myString", "Hello"), ("myDint.1", True)]

# write the values
ret = comm.Write(write_data)

# print the status of the writes
for r in ret:
    print(r.TagName, r.Value, r.Status)

prints:

myDint 100 Success
myString Hello Success
myDint.1 (2, -1) Success

dmroeder added a commit that referenced this issue May 15, 2023
This reverts a previous commit to deal with data types that were assinged
a value of 0.  This will have to be investigated further.

Fixes #224
@dmroeder
Copy link
Owner

Yeah, that is weird. The latest should fix it.

@Destination2Unknown
Copy link
Author

Yeah, that is weird. The latest should fix it.

This case is still incorrect:

from pylogix import PLC

with PLC() as comm:
    comm.IPAddress = "192.168.133.199" #incorrect IP address

    write_data = [("tag1", 100), ("tag2", 6.45), ("tag3", True)]

    # write the values
    ret = comm.Write(write_data)

    # print the status of the writes
    for r in ret:
        print(r.TagName, r.Status)

prints:

tag2 Unknown error timed out
6.45 Unknown error timed out

@dmroeder
Copy link
Owner

I see, somehow, I missed that comment originally

@dmroeder
Copy link
Owner

Okay, that one should be fixed on that branch too.

I considered whether to return the value that was written or the None. I went with value intended to write because that is what would be returned on a successful write. Though I could be convinced otherwise.

@Destination2Unknown
Copy link
Author

Okay, that one should be fixed on that branch too.

I considered whether to return the value that was written or the None. I went with value intended to write because that is what would be returned on a successful write. Though I could be convinced otherwise.

I strongly favour returning None because it would make it crystal clear that there was an issue.
Otherwise, it could lead to confusion when a write value is returned despite not being sent successfully.

Thank you for your assistance.
This library is exceptional, and the support provided is truly outstanding!

@dmroeder
Copy link
Owner

Sold! I'll return None. I appreciate your feedback and testing.

@Destination2Unknown
Copy link
Author

Looks good to me, thanks again!

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

No branches or pull requests

2 participants