5
\$\begingroup\$

What I made

I used Tkinter to write a GUI to run some HPLC pumps for my work. The application sends some messages to the pumps via serial connections, reads a response, then logs data to a csv file. main window of program

I use Matplotlib to plot the data in real time, and to let the user plot several sets of data at once.

This is mostly a personal project I am doing in my free time at work, but I am trying to make a good-practices learning experience out of it. Nobody else is going to develop this code, but I am trying to write it to a shareable standard, such that I could include this in a portfolio of sorts one day.

I have already linted the code up to PEP8, with maybe an exception or two.

How it is used

The typical workflow is to start a test and afk while it logs and visualizes data. Occasionally, I like to plot several sets of data for comparison. The menu bar > make new plot functionality lets me do this. The plotter window takes file paths and titles to use in the plot legend. Because I make the same plot often (or versions of it) it is helpful to be able to store those file paths and titles (rather than the image itself). I accomplish this by pickling a list to a ".plt" file that I can unpickle later to repopulate the plotter window. enter image description here

I am the only person using this program, but I am trying to build it in such a way that others could learn my job function and use it too. To this end, I am trying to make it pretty and intuitive.

Since you won't have the equipment connected some buttons won't work, but most of the app should be available for prodding.

My main areas of concern

  • Readability
    • I did lint the code, but it still hurts my eyes in some spots... - what do?
  • Design
    • UI elements and logic elements are kind of mixed - what do? it's not a massive app, but most lines of code are setting up GUI elements
    • I would like to separate the UI and logic more, but I'm not sure how, or if doing so would be "astronaut architecture" for a project of this size
  • Object-orientedness
    • I am unsure of if I am using self.foo too much or not enough
    • I can't help but feel the way I am inheriting objects and using them is wrong, such as here
    • eg. I don't fully grasp the meaning of what this is doing
    •    def __init__(self, parent, *args, **kwargs):
             tk.Frame.__init__(self, parent, *args, **kwargs)
             ...```
      
  • Portability
    • the code currently imports from the Winsounds module which is only available for Windows machines :(
    • the startup time (and file size) for one-file bundles seems to rapidly grow with additional module imports. is using python in this way to make executables just a bad practice?
    • I want someone else to be able to drop this file on their computer and it "just works"

The layout of the code

  • core.py is the entry point
    • imports then creates an instance of MainWindow
    • has a thread_pool_executor attribute to handle a blocking loop we make in MainWindow
  • mainwindow.py is the bulk of the application
    • imports then creates an instance of MenuBar
    • has some tkinter widgets for user input / data visualization
  • menubar.py
    • the menubar for the MainWindow
    • lets user set project directory
    • can create a Plotter object (new tkinter toplevel)
  • plotter.py
    • imports and makes a bunch of SeriesEntry objects (customer tkinter widget)
    • can pickle/unpickle the contents of the SeriesEntry objects
  • seriesentry.py
    • custom tkinter widget for selecting csv files and which column of data to be plotted

The code

Here's my GitHub repo for the code.

There's some sample csv data in the demo/sample_data repo folder if you want to try the plotting feature

Thanks for your time!

Here's mainwindow.py

"""The main window of the application.
  - imports then creates an instance of MenuBar
  - has some tkinter widgets for user input / data visualization
"""


import csv  # logging the data
from datetime import datetime  # logging the data
import matplotlib.pyplot as plt  # plotting the data
from matplotlib.animation import FuncAnimation
from matplotlib.backends.backend_tkagg import FigureCanvasTkAgg
from matplotlib.backends.backend_tkagg import NavigationToolbar2Tk
from matplotlib.ticker import MultipleLocator
from pandas import DataFrame, read_csv  # reading data from csv
import os  # handling file paths
import serial  # talking to the pumps
import sys  # handling file paths
import tkinter as tk  # GUI
from tkinter import ttk
import time  # sleeping
from winsound import Beep  # beeping when the test ends

from menubar import MenuBar


class MainWindow(tk.Frame):
    def __init__(self, parent, *args, **kwargs):
        tk.Frame.__init__(self, parent, *args, **kwargs)
        self.parent = parent

        # define test parameters
        self.port1 = tk.StringVar()  # COM port for pump1
        self.port2 = tk.StringVar()  # COM port for pump2
        self.timelimit = tk.DoubleVar()
        self.failpsi = tk.IntVar()
        self.chem = tk.StringVar()
        self.conc = tk.StringVar()
        self.savepath = tk.StringVar()  # output directory
        self.project = tk.StringVar()  # used for window title
        self.plotpsi = tk.StringVar()  # for which pump's data to plot
        self.plotstyle = tk.StringVar()

        # set initial
        self.paused = True
        self.timelimit.set(90)
        self.failpsi.set(1500)
        self.savepath.set(os.getcwd())
        self.plotpsi.set('PSI 2')
        self.plotstyle.set('seaborn-colorblind')
        self.outfile = f"{self.chem.get()}_{self.conc.get()}.csv"
        self.build_window()

    def build_window(self):
        """Make all the tkinter widgets"""
        self.menu = MenuBar(self)
        # build the main frame
        self.tstfrm = tk.Frame(self.parent)
        self.entfrm = tk.LabelFrame(self.tstfrm, text="Test parameters")
        # this spacing is to avoid using multiple labels
        self.outfrm = tk.LabelFrame(self.tstfrm,
            text="Elapsed,            Pump1,             Pump2")
        self.cmdfrm = tk.LabelFrame(self.tstfrm, text="Test controls")

        # define the self.entfrm entries
        self.p1 = ttk.Entry(
            master=self.entfrm,
            width=14,
            textvariable=self.port1,
            justify=tk.CENTER
            )
        self.p2 = ttk.Entry(
            master=self.entfrm,
            width=14,
            textvariable=self.port2,
            justify=tk.CENTER
            )
        self.tl = ttk.Entry(
            master=self.entfrm,
            width=30,
            justify=tk.CENTER,
            textvariable=self.timelimit
            )
        self.fp = ttk.Entry(
            master=self.entfrm,
            width=30,
            justify=tk.CENTER,
            textvariable=self.failpsi
            )
        self.ch = ttk.Entry(
            master=self.entfrm,
            width=30,
            justify=tk.CENTER,
            textvariable=self.chem
            )
        self.co = ttk.Entry(
            master=self.entfrm,
            width=30,
            justify=tk.CENTER,
            textvariable=self.conc
            )
        self.strtbtn = ttk.Button(
            master=self.entfrm,
            text="Start",
            command=self.init_test
            )

        # grid entry labels into self.entfrm
        self.comlbl = ttk.Label(master=self.entfrm, text="COM ports:")
        self.comlbl.grid(row=0, sticky=tk.E)
        ttk.Label(
            master=self.entfrm,
            text="Time limit (min):"
            ).grid(row=1, sticky=tk.E)
        ttk.Label(
            master=self.entfrm,
            text="Failing pressure (psi):"
            ).grid(row=2, sticky=tk.E)
        ttk.Label(
            master=self.entfrm,
            text="Chemical:"
            ).grid(row=3, sticky=tk.E)
        ttk.Label(
            master=self.entfrm,
            text="Concentration:"
            ).grid(row=4, sticky=tk.E)

        # grid entries into self.entfrm
        self.p1.grid(row=0, column=1, sticky=tk.E, padx=(9, 1))
        self.p2.grid(row=0, column=2, sticky=tk.W, padx=(5, 3))
        self.tl.grid(row=1, column=1, columnspan=3, pady=1)
        self.fp.grid(row=2, column=1, columnspan=3, pady=1)
        self.ch.grid(row=3, column=1, columnspan=3, pady=1)
        self.co.grid(row=4, column=1, columnspan=3, pady=1)
        self.strtbtn.grid(row=5, column=1, columnspan=2, pady=1)
        cols = self.entfrm.grid_size()
        for col in range(cols[0]):
            self.entfrm.grid_columnconfigure(col, weight=1)

        # build self.outfrm PACK
        scrollbar = tk.Scrollbar(self.outfrm)
        self.dataout = tk.Text(
            master=self.outfrm,
            width=39,
            height=12,
            yscrollcommand=scrollbar.set,
            state='disabled'
            )
        # TODO: try calling tk.Scrollbar(self.outfrm) directly
        scrollbar.config(command=self.dataout.yview)
        scrollbar.pack(side=tk.RIGHT, fill=tk.Y)
        self.dataout.pack(fill=tk.BOTH)

        # build self.cmdfrm 4x3 GRID
        self.runbtn = ttk.Button(
            master=self.cmdfrm,
            text="Run",
            command=lambda: self.run_test(),
            width=15
            )
        self.endbtn = ttk.Button(
            master=self.cmdfrm,
            text="End",
            command=lambda: self.end_test(),
            width=15
            )
        self.runbtn.grid(row=1, column=1, padx=5, pady=2, sticky=tk.W)
        self.endbtn.grid(row=1, column=2, padx=5, pady=2, sticky=tk.E)
        tk.Label(
            master=self.cmdfrm,
            text="Select data to plot:"
            ).grid(row=0, column=0, padx=5)
        tk.Radiobutton(
            master=self.cmdfrm,
            text="PSI 1",
            variable=self.plotpsi,
            value='PSI 1'
            ).grid(row=0, column=1, padx=5)
        tk.Radiobutton(
            master=self.cmdfrm,
            text="PSI 2",
            variable=self.plotpsi,
            value='PSI 2'
            ).grid(row=0, column=2, padx=5)

        # disable the controls to prevent starting test w/o parameters
        if self.paused:
            for child in self.cmdfrm.winfo_children():
                child.configure(state="disabled")

        # set up the plot area
        self.pltfrm = tk.LabelFrame(
            master=self.tstfrm,
            text=("Style: " + self.plotstyle.get())
            )

        self.fig, self.ax = plt.subplots(figsize=(7.5, 4), dpi=100)
        plt.subplots_adjust(left=0.10, bottom=0.12, right=0.97, top=0.95)
        # TODO: explicitly clarify some of these args
        self.canvas = FigureCanvasTkAgg(self.fig, master=self.pltfrm)
        toolbar = NavigationToolbar2Tk(self.canvas, self.pltfrm)
        toolbar.update()
        self.canvas.get_tk_widget().pack()
        self.ani = FuncAnimation(self.fig, self.animate, interval=1000)

        # grid stuff into self.tstfrm
        self.entfrm.grid(row=0, column=0, sticky=tk.NSEW, pady=2)
        self.pltfrm.grid(row=0, column=1, rowspan=3, sticky=tk.NSEW, padx=2)
        self.outfrm.grid(row=1, column=0, sticky=tk.NSEW, pady=2)
        self.cmdfrm.grid(row=2, column=0, sticky=tk.NSEW, pady=2)

        # widget bindings
        self.co.bind("<Return>", self.init_test)
        self.comlbl.bind("<Button-1>", lambda _: self.findcoms())

        self.tstfrm.grid(padx=3)
        self.findcoms()
        self.ch.focus_set()

    def findcoms(self):
        """Looks for COM ports and disables the controls if two aren't found"""
        self.to_log("Finding COM ports...")
        ports = ["COM" + str(i) for i in range(15)]
        useports = []
        for i in ports:
            try:
                if serial.Serial(i).is_open:
                    self.to_log(f"Found an open port at {i}")
                    useports.append(i)
                    serial.Serial(i).close
            except serial.SerialException:
                pass
        if useports == []:
            self.to_log("No COM ports found...")
            self.to_log("Click 'COM ports:' to try again.")
            useports = ["??", "??"]
        try:
            self.port1.set(useports[0])
            self.port2.set(useports[1])
            if self.port1.get() == "??" or self.port2.get() == "??":
                self.strtbtn['state'] = ['disable']
            else:
                self.strtbtn['state'] = ['enable']

        except IndexError:
            pass
        except AttributeError:
            pass

    def init_test(self):
        """Collects all the user data from the GUI widgets"""
        self.port1.set(self.p1.get())
        self.port2.set(self.p2.get())
        self.timelimit.set(self.tl.get())
        self.failpsi.set(self.fp.get())
        self.chem.set(self.ch.get())
        self.conc.set(self.co.get())

        self.outfile = f"{self.chem.get()}_{self.conc.get()}.csv"
        self.psi1, self.psi2, self.elapsed = 0, 0, 0
        # the timeout values are an alternative to using TextIOWrapper
        self.pump1 = serial.Serial(self.port1.get(), timeout=0.01)
        print(f"Opened a port at {self.port1.get()}")
        self.pump2 = serial.Serial(self.port2.get(), timeout=0.01)
        print(f"Opened a port at {self.port2.get()}")

        # set up output file
        outputpath = os.path.join(self.savepath.get(), self.outfile)
        print(f"Creating output file at {outputpath}")
        with open(os.path.join(self.savepath.get(), self.outfile), "w") as f:
            csv.writer(f, delimiter=',').writerow(
                                                    [
                                                     "Timestamp",
                                                     "Seconds",
                                                     "Minutes",
                                                     "PSI 1",
                                                     "PSI 2"
                                                     ]
                                                  )
        # disable the entries for test parameters
        for child in self.entfrm.winfo_children():
            child.configure(state="disabled")
        # enable the commands for starting/stopping the test
        for child in self.cmdfrm.winfo_children():
            child.configure(state="normal")

    def to_log(self, msg):
        """Logs a message to the Text widget in MainWindow's outfrm"""
        self.dataout['state'] = 'normal'
        self.dataout.insert('end', f"{msg}" + "\n")
        self.dataout['state'] = 'disabled'
        self.dataout.see('end')

    def end_test(self):
        """Stops the pumps and closes their COM ports, then swaps the button
        states for the entfrm and cmdfrm widgets"""
        self.paused = True
        self.pump1.write('st'.encode())
        self.pump1.close()
        self.pump2.write('st'.encode())
        self.pump2.close()
        msg = "The test finished in {0:.2f} minutes".format(self.elapsed/60)
        self.to_log(msg)
        for child in self.entfrm.winfo_children():
            child.configure(state="normal")
        for child in self.cmdfrm.winfo_children():
            child.configure(state="disabled")

    def run_test(self):
        """Submits a test loop to the thread_pool_executor"""
        if self.paused:
            self.pump1.write('ru'.encode())
            self.pump2.write('ru'.encode())
            self.paused = False
            # let the pumps warm up before we start recording data
            time.sleep(3)
            self.parent.thread_pool_executor.submit(self.take_reading)

    def take_reading(self):
        """loop to be handled by the thread_pool_executor"""
        starttime = datetime.now()
        while (
         (self.psi1 < self.failpsi.get() or self.psi2 < self.failpsi.get())
         and self.elapsed < self.timelimit.get()*60
         and not self.paused
         ):
            rn = time.strftime("%I:%M:%S", time.localtime())
            self.pump1.write("cc".encode())
            self.pump2.write("cc".encode())
            time.sleep(0.1)
            self.psi1 = int(self.pump1.readline().decode().split(',')[1])
            self.psi2 = int(self.pump2.readline().decode().split(',')[1])
            thisdata = [
                        rn,
                        self.elapsed,  # as seconds
                        '{0:.2f}'.format(self.elapsed/60),  # as minutes
                        self.psi1,
                        self.psi2
                        ]

            outputpath = os.path.join(self.savepath.get(), self.outfile)
            with open((outputpath), "a", newline='') as f:
                csv.writer(f, delimiter=',').writerow(thisdata)
            nums = ((self.elapsed/60), self.psi1, self.psi2)
            logmsg = ("{0:.2f} min, {1} psi, {2} psi".format(nums))
            self.to_log(logmsg)
            time.sleep(0.9)
            self.elapsed = (datetime.now() - starttime).seconds

        if not self.paused:
            self.end_test()
            for i in range(3):
                Beep(750, 500)
                time.sleep(0.5)

    def animate(self, i):
        """The animation function for the current test's data"""
        try:
            data = read_csv(os.path.join(self.savepath.get(), self.outfile))
        except FileNotFoundError as e:
            data = DataFrame(data={'Minutes': [0], 'PSI 1': [0], 'PSI 2': [0]})

        # TODO: this plt stuff can probably go elsewhere
        plt.rcParams.update(plt.rcParamsDefault)  # refresh the style
        # https://stackoverflow.com/questions/42895216
        with plt.style.context(self.plotstyle.get()):
            self.pltfrm.config(text=("Style: " + self.plotstyle.get()))
            self.ax.clear()
            self.ax.set_xlabel("Time (min)")
            self.ax.set_ylabel("Pressure (psi)")
            self.ax.set_ylim(top=self.failpsi.get())
            self.ax.yaxis.set_major_locator(MultipleLocator(100))
            self.ax.set_xlim(left=0, right=self.timelimit.get())

            y = data[self.plotpsi.get()]
            x = data['Minutes']
            self.ax.plot(x, y, label=(f"{self.chem.get()}_{self.conc.get()}"))

            self.ax.grid(color='grey', alpha=0.3)
            self.ax.set_facecolor('w')
            self.ax.legend(loc=0)

\$\endgroup\$
1
  • \$\begingroup\$ Dude, this is really impressive. \$\endgroup\$
    – Reinderien
    Apr 12, 2020 at 17:44

1 Answer 1

2
\$\begingroup\$

Data vis considerations

Currently you seem to have a fixed time-axis range. Consider scaling it as time passes. I have also written an industrial control project with some features similar to this, but my time display was rendered differently and could give you some ideas:

  • The newest data actually appear at the right edge of the graph
  • The time axis ranges from a negative earliest-seen time to zero, interpretable as "seconds/minutes/etc. ago"
  • The time axis dynamically changes its units based on how much time has elapsed

Unfortunately the code wouldn't be of much use to you since it's in Mono/GTK2, but the algorithm could be used.

Also, your data appear to be exponential in nature. If that holds true I would suggest making the vertical axis logarithmic.

p.s. once your graph is properly scaled you will find that the legend position may need to move to either upper-left or lower-right to avoid occluding your curves.

Mixing logic and presentation

MainWindow was doing good, adhering to GUI-only tasks - right up until findcoms. This is pretty clearly a business logic concern and should be separated into a different class and/or module.

init_test is a big mix of UI and logic, which you should also attempt to decouple.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Thanks for your consideration! I’ll play around with putting the vertical axis on a log scale and see how it presents. Since this is pressure vs time data though a logarithmic unit might be confusing to interpret. I’ll look into cleaning up findcoms/ init_test. I think a simple import statement should let me separate the second half of mainwindow into just test logic. Thanks for your time! \$\endgroup\$
    – teauxfu
    Apr 12, 2020 at 18:10
  • 1
    \$\begingroup\$ Log scales are easy enough to interpret as long as some basic rules are followed - use a radix of 10; show sub-ticks; show a pale sub-grid and a normal-opacity primary grid; etc. This should be easy to do with matplotlib. \$\endgroup\$
    – Reinderien
    Apr 12, 2020 at 18:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.