Bug fix and additional updates to 81_simple_gui.py file

[GitHubDragonFly]

Short description of change

Bug fix + new feature + additional error checking

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [x] I have read the docs/CONTRIBUTING.md document.
• [x] My code follows the code style of this project.
• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [x] I have read tests/README.md.
• [ ] I have added tests to cover my changes.
• [ ] All new and existing tests passed.

What is the change?

• Use semicolon for multiple tag separation
  • This was done to replace comma as a separator of multiple tags
  • Multi-dimensional arrays use comma so it was conflicting, ex. DINTArray[0,1] was seen as 2 tags -> "DINTArray[0" and "1]"
• Added additional error checking
• Updating tag value was re-pointed to start a connection thread instead of performing comm_check() itself when the connection is lost. Code was adjusted so the updating can continue once the connection is re-established.
• New feature: allow reading multiple elements of an array, needs to be entered as a single tag in the following format: tagName[startIndex]{elementCount}, where "startIndex" is either of: x or x,y or x,y,z (single or multi dimensional arrays)

What does it fix/add?

Bug that was introduced by using comma for separating multiple tags

Test Configuration

• PLC Model
• PLC Firmware
• pylogix version 0.7.8
• python version 3.9
• OS type and version Windows 10 v2004

[TheFern2]

Actually this brings up a good point. I think the UI itself should have information about how to read multiple tags either above the input box or a instructions pop up.

At first glance it might not be too obvious that multiple tag read is supported through the ui.

[GitHubDragonFly]

The label above the listbox states "Tag(s) to Read".

I have just made the following changes for further clarification:

• Set the app start up with multiple tags
• Comments explaining the usage moved to the top of file

Users should be able to edit the file and see the comments.

[GitHubDragonFly]

Another comment could possibly be added to the file, similar to this:

See the screenshots here: https://github.com/dmroeder/pylogix/issues/156

[GitHubDragonFly]

All together, this example app is intended to be used for quick testing.

Anybody looking for any prolonged usage would probably need to modify the app to their liking.

[GitHubDragonFly]

If you want more description visible on the GUI then consider changing the text of the label above the listbox, which now states "Tag(s) to Read" but can be changed to anything else, maybe "Single tag or Multiple tags separated by semicolon".

[TheFern2]

Yeah I'll see how it looks when I get power back, but I think just a quick example(s) like on the comment would be better on the UI. We're on rotating blackouts in Texas right now.

[GitHubDragonFly]

There are 2 new checkboxes added to allow saving tags list and tag(s) values to separate files within the app's folder.

[TheFern2]

Thanks for the PR, a couple of bugs:

• STRING tags not reading
• Complex tags work individually but not when reading multiple tags, see UDTCombinedArray screenshot below
• Save tags list checkbox does not work, I get a tooltip which doesn't let me click on the checkbox

Some improvements that the GUI could benefit from, these are not bugs and are optional, as long as bugs are fixed we can merge PR:

• Add tag examples right before the input Tags(s) to read, or as a quick separate popup, especially since the multiple elements from an array isn't a format that pylogix users are used to. This will improve UX instead of going to the file, we do have quite a few users that are just PLC guys and might be unfamiliar with reading python code.
• Text box is quite small once you read say more than N tags
• Datalogging file could use a header line with column names

[GitHubDragonFly]

This app does nothing else but pass tag(s) to the library for reading, and this is based on the examples you have in the repo.

1) It reads STRING values for me 2) Complex tags not being allowed to be packed in a list is the library issue 3) Save tags list checkbox does work since it is set to put a check mark after the tooltip is shown (just click the tooltip or outside of it and look at the checkbox).

Here is what I can suggest:

• If there is an exception thrown while reading then it should now show in the tag value list box
• If there is no exception and no value showing then it normally means that tag is incorrect or not supported
• You need to consider updating your lgx_comm.py file and take a look at the error I posted here: https://github.com/dmroeder/pylogix/issues/156
• The mentioned error seems to be randomly happening, dependent on when the "Stop Update" is clicked, probably not letting the library process the last request.

[TheFern2]

This app does nothing else but pass tag(s) to the library for reading, and this is based on the examples you have in the repo.

1. It reads STRING values for me

2. Complex tags not being allowed to be packed in a list is the library issue

3. Save tags list checkbox does work since it is set to put a check mark after the tooltip is shown (just click the tooltip or outside of it and look at the checkbox).

Here is what I can suggest:

* If there is an exception thrown while reading then it should now show in the tag value list box

* If there is no exception and no value showing then it normally means that tag is incorrect or not supported

* You need to consider updating your lgx_comm.py file and take a look at the error I posted here: #156

* The mentioned error seems to be randomly happening, dependent on when the "Stop Update" is clicked, probably not letting the library process the last request.

Fair enough on the complex tags, I actually don't use them much on my projects, but I thought they were implemented on lists. I was able to read Strings not unsure what happened earlier. Error handling is much better now. Only issue remaining is the checkbox, see video below I clicked all over and nothing. I am on Linux, so it could be an OS issue, but the other two checkboxes seem to check just fine.

https://www.dropbox.com/s/ui6m76br87ccek0/2021-02-19_15-13-17.mov?dl=0

[GitHubDragonFly]

Try swapping the bottom 2 lines in this function:

def save_tags_list(event, chbSaveTags): if checkVarSaveTags.get() == 0: popup_menu_save_tags_list.post(event.x_root, event.y_root) checkVarSaveTags.set(1)

It should set a check mark before it pops up the tooltip. If that doesn't work then Linux just doesn't support setting the state of the tkinter checkbox programmatically. The only solution for this would be to remove the binding and not show the tooltip (users would need to be aware that tags list is saved only after the checkbox is checked and later the Get Tags button is clicked).

You also seem to be getting those errors I mentioned displayed in the tag value box. If you try updating the lgx_comm.py file with the solution I mentioned then you will know if it works or not.

[GitHubDragonFly]

Here is another possible solution:

replace checkVarSaveTags.set(1) with chbSaveTags.select()

[GitHubDragonFly]

This last commit seems to work properly in Linux.

[GitHubDragonFly]

This last commit is an attempt to eliminate those odd errors causing the exception thrown when reading tags.

You might not need to make any changes to the lgx_comm.py file after all.

[TheFern2]

This last commit is an attempt to eliminate those odd errors causing the exception thrown when reading tags.

You might not need to make any changes to the lgx_comm.py file after all.

Awesome everything looks good, thanks for the hard work. Lots of people will benefit from this GUI for quick testing for sure. :+1: