dmroeder / pylogix

Read/Write data from Allen Bradley Compact/Control Logix PLC's
Apache License 2.0
599 stars 182 forks source link

Simple GUI Example - Updated #156

Closed GitHubDragonFly closed 3 years ago

GitHubDragonFly commented 3 years ago

This is just in case if you might be interested in updated version of your 80_simple_gui.py example.

It goes through the Device Discovery as well as Get Tags and shows the results in separate list boxes. Also, it has 2 buttons to start and stop updating the tag.

It worked fine for me in Windows 10 and with python3.9.

PR created with the latest updates.

Do close this non-issue whenever you wish.

Attached are pictures of the new GUI interface.

Pylogix GUI 1

Pylogix GUI 2

TheFern2 commented 3 years ago

Nice work! please submit PR, and name file 81_simple_gui.py just to keep both files. Does it work with python2?

dmroeder commented 3 years ago

I agree with @TheFern2, rename it so the original stays intact.

GitHubDragonFly commented 3 years ago

PR has been created and I have removed the file from my initial post.

I don't know if it runs under python2 but in python3 everything seems to work just fine.

Additional feature was added to enable the right-click on the Tag To Poll entry box and show "Paste" menu when the clipboard has contents. This paste feature doesn't really fully behave as the standard text box in Windows.

You might need to check your original file since the form title shows "tk" instead of "Production Count" and maybe also compare the Tkinter import to the one I submitted since it should support python 2 and 3.

GitHubDragonFly commented 3 years ago

Some of the variable and function names, in the file I submitted, might need to be changed to start with lower letter instead. I also left my IP Address, processor slot and tag name, not sure if you want to have it the same as in the original version (these can be changed via GUI anyway).

If you can change it before merging it then good, otherwise I can update it and create another PR.

I did experience couple of random crashes after clicking the "Start Update" button, pointing to eip.py file and suggesting that the buffer size needed was 49 but it was currently set as 24. I am not really sure why this was happening.

GitHubDragonFly commented 3 years ago

I have already updated the file and can submit another PR once you are to handle the current PR.

There is another new feature, which is to allow copying the IP address from the discovered devices list box and paste it to the IP Address entry box.

GitHubDragonFly commented 3 years ago

As for reading and listing UDT members, I have tried using pycomm3 driver and was successful to the point.

There is a post on pycomm3 repo, where I am looking for some input for creating recurring code to penetrate "n" levels of UDT, so if either of you could pitch in, that would be nice:

https://github.com/ottowayi/pycomm3/issues/98

GitHubDragonFly commented 3 years ago

I did manage to resolve the recurring thing mentioned in the previous post.

The updated file was posted in the mentioned issue and it is using both pylogix and pycomm3 at the same time. Pylogix is being used for the Device Discovery until the official release of pycomm3 includes that functionality.

I will go ahead and close this issue in your repo.

GitHubDragonFly commented 3 years ago

Just if you might be interested, here is a screenshot of a random error that points to lgx_comm.py file. Maybe a size check of the ret_data might be introduced before unpacking is done.

Pylogix Error

This seems to work but I am not familiar with all your code:

def _getBytes(self, data, connected):
    """
    Sends data and gets the return data, optionally asserting data size limit
    """
    if self.ConnectionSize is not None and len(data) > self.ConnectionSize:
        raise BufferError("ethernet/ip _getBytes output size exceeded: %d bytes" % len(data))
    try:
        self.Socket.send(data)
        ret_data = self.recv_data()
        if ret_data:
            if connected:
                if len(ret_data) > 48:
                    status = unpack_from('<B', ret_data, 48)[0]
                else:
                    status = 1
            else:
                if len(ret_data) > 42:
                    status = unpack_from('<B', ret_data, 42)[0]
                else:
                    status = 1

            return status, ret_data
        else:
            return 1, None
    except (socket.gaierror):
        self.SocketConnected = False
        return 1, None
    except (IOError):
        self.SocketConnected = False
        return 7, None
GitHubDragonFly commented 3 years ago

Can any of you let me know if you do not want to accept the latest pull request?

This so I can close the PR and post that file as it is in my own repo.

TheFern2 commented 3 years ago

I haven't got around to test it yet, busy week. Usually a normal workflow when working on PRs, is to fork a repo. Make a change push to origin (your repo), then make a PR. Then you can also pull from upstream (original repo) to get updated.

origin and upstream are just remote urls btw.

You shouldn't have to wait on a PR to get pulled to put the file on your fork.

If a PR isn't close yet, then is safe to assume it will be reviewed as soon as we have time to test it.

GitHubDragonFly commented 3 years ago

Sounds good and I will leave the PR open.

I think that I'm done with it unless I find some bug.

GitHubDragonFly commented 3 years ago

I deleted and re-created the fork so a new PR was filed. The picture of the GUI in the first post here is the latest one.

Some GUI code was re-grouped since new frames were introduced.

Hopefully you can manage to follow the flow of all changes.

TheFern2 commented 3 years ago

Thanks, looks like latest pr has no conflicts, and I can see the automated diff on the new changes. I'll get it tested and merged asap.

GitHubDragonFly commented 3 years ago

The connection seems to be hogging GUI upon startup and when there is no network available, so here is what you can try changing when doing testing (it is only a few lines of code that require change, the main one being root.after(...)):

def comm_check(): global comm global connected

ip = selectedIPAddress.get()
port = int(selectedProcessorSlot.get())

if (not connected or comm.IPAddress != ip or comm.ProcessorSlot != port or changePLC.get() == 1):
    if not comm is None:
        comm.Close()
        comm = None

    comm = PLC()
    comm.IPAddress = ip

    if checkVar.get() == 0:
        comm.ProcessorSlot = port
        comm.Micro800 = False
    else:
        comm.Micro800 = True

    lbConnectionMessage.delete(0, 'end')
    lbErrorMessage.delete(0, 'end')

    plcTime = comm.GetPLCTime()

    if plcTime.Value is None:
        if btnStop['state'] == 'disabled':
            btnStart['state'] = 'disabled'
        lbConnectionMessage.insert(1, 'Not Connected')
        lbErrorMessage.insert(1, plcTime.Status)
        connected = False
        root.after(5000, start_connection)
    else:
        lbConnectionMessage.insert(1, 'Connected')
        if btnStop['state'] == 'disabled':
            btnStart['state'] = 'normal'
        connected = True

changePLC.set(0)
GitHubDragonFly commented 3 years ago

The code above will be flashing connection and error messages every 5 seconds, so in order to avoid seeing that just move the GetPLCTime() function above the clearing of the listboxes, like this:

    plcTime = comm.GetPLCTime()

    lbConnectionMessage.delete(0, 'end')
    lbErrorMessage.delete(0, 'end')
GitHubDragonFly commented 3 years ago

I just don't like keeping you busy testing but there was a bug that needed to be fixed.

Some other code was also changed and a new feature was added.

I certainly hope that this is the last update for this particular file.

TheFern2 commented 3 years ago

Let's keep the conversation within the PR.