jetperch / pyjoulescope_ui

Joulescope graphical user interface
https://www.joulescope.com
Apache License 2.0
87 stars 26 forks source link

UI does not terminate cleanly #129

Closed mliberty1 closed 1 year ago

mliberty1 commented 3 years ago

Platform: Linux (reported, but also happens on Windows, not yet tested on Mac) Version: 0.9.7

See forum. Thank you @MortenGuldager!

Description When you exit the Joulescope UI in some configurations, it does not shut down cleanly. Threads continue to execute. These threads may interfere with subsequent Joulescope UI launches.

Expected Behavior All Joulescope UI threads should terminate when you exit the Joulescope UI.

Steps to reproduce

  1. remove ~/.joulescope
  2. start joulescope, either binary dist or the pip installed one
  3. select View → Oscilloscope
  4. enable View → Multimeter (the 5th line) - at this point you see both graphs and numbers
  5. drag out the multimeter to become a separate window
  6. click File → Exit
mliberty1 commented 3 years ago

It looks like the threads still respond correctly to kill. So for linux, you can:

  1. Type CTRL-Z in the terminal where you ran joulescope_launcher
  2. Type jobs
  3. Look for the Joulescope UI process id.
  4. Type kill %{job_id}, such as kill %1

Alternatively: killall -9 joulescope_launcher

mliberty1 commented 2 years ago

Upon investigation, the problem lies in how the Joulescope UI uses QDockWidget. Instead of letting QDockWidgets manage themselves, the UI overrides closeEvent, which can ignore the close event. When one dock window shuts down, this additional behavior prevents other dock windows from shutting down, which prevents the application from closing.

The purpose of this override is to support singleton widgets, which includes all existing widgets except SingleValueWidget. The goal of singletons was to make the UI easy to use. No need to have two multimeters and better just to toggle. We now have other features to simplify widget management, such as profiles. Going forward, the UI will support multiple simultaneous connected Joulescopes, so singletons will definitely not be a good choice.

Since fixing requires a fairly major restructuring of the widgets which needs to be done as part of multiple device support, we will defer this issue until we add multiple device support.

mliberty1 commented 1 year ago

This particular issue was fixed in 1.0.0 by moving to the Advanced Docking System.

I did see a few times that the UI did not close cleanly leading up to the 1.0.0 release. I have not yet seen it with the 1.0.0 release, but it's possible (likely?) that a similar issue exists. However, since the cause will be different, let's close this issue. We can open a new issue if we find problems with 1.0.0 shutting down.