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

Qt widget wrapper #480

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from
Draft

Qt widget wrapper #480

wants to merge 15 commits into from

Conversation

MarcusZuber
Copy link
Member

@MarcusZuber MarcusZuber commented Apr 4, 2022

First attempt to write a qt-widget-wrapper that can be applied to a Parameterizable.

The code is still quite chaotic but in the first tests it did what it should be.

An example session:

import logging
from inspect import iscoroutinefunction
import concert
concert.require("0.30.1")

from concert.quantities import q
from concert.session.utils import ddoc, dstate, pdoc, code_of

from concert.devices.motors.dummy import LinearMotor
from concert.gui.parameterizable import ParameterizableWidget

LOG = logging.getLogger(__name__)

lin_motor = await LinearMotor()
lin_motor_widget = ParameterizableWidget(lin_motor)

Then lin_motor_widget.show() opens the widget. Can be also used as a general widget in qt layouts etc.

TODOs / Questions:

  • How to handle Properties with arbitrary data types? Maybe add a string-to-property function to the property?
    -> I will add a string-to-property function to the Property-Widget
  • How to add functions? Create subclasses for each device?
    This would require a subclass for each device and would lead to a bridge like implementation (if seen from the gui side).
    Maybe a simple way to add buttons connected to functions and a factory that sets this for the most common devices/experiments would be a more clean solution (also with less work...)
  • Test performance of (ugly manual) event-loop. (In a standard experimental setup no problems where found.)

In concert.gui.parameterizable there is ParameterWidget (+ StateWidget QuantityWidget SelectionWidget that inherit from ParameterWidget). This widget implements a Widget for a (async) setter/getter for a Parameter (Quantitiy....). The setting/getting runs detatched (with the qasynv.asyncSlot). It also features the signals

setter_started = pyqtSignal()
setter_finished = pyqtSignal()
setter_error = pyqtSignal(Exception)

getter_started = pyqtSignal()
getter_finished = pyqtSignal()
getter_error = pyqtSignal(Exception)

that are emitted when a setter/getter is called, finished and when an Exception is raised within.

The ParameterizableWidget adds all parameter (etc.) widgets to a new Widget. The paramter-widgets are in the dictionary params. This allows to connect the setter/getter signals to any arbitrary function.
(E.g. in the example session above)

def print_done():
   print("done")

lin_motor_widget.params['position'].setter_finished.connect(print_done)

All widgets are "functional" and there are many options to make the look more appealing.

@MarcusZuber MarcusZuber self-assigned this Apr 4, 2022
@MarcusZuber MarcusZuber changed the title First qt-widget-wrapper implementation Qt widget wrapper Apr 4, 2022
@MarcusZuber MarcusZuber marked this pull request as draft April 4, 2022 07:51
@MarcusZuber
Copy link
Member Author

MarcusZuber commented Apr 5, 2022

With the Widgets one can build a standalone application like (maybe the setting of the asyncio-eventloop is required)

from PyQt5.QtGui import QApplication
from concert.devices.samplechangers.dummy import SampleChanger
from concert.gui.parameterizable import ParameterizableWidget

if __name__ == '__main__':
   app = QApplication([]) # or args etc.
   changer = SampleChanger()
   changer_widget = ParameterizableWidget(changer)
   changer_widget.show() # or an arbitrary combination in layouts etc.
   app.exec()

I also wanted to have the possibility to open a Widget from a running concert session. Therefore, I added this manual "qt event loop". The possibility to open a widget in a running session is required, if devices do not allow access from multiple connections (or when parameters/state is handled in concert -> that's why I don't like the transition decorator).
Also for checking within a running session if a device is working correctly (without setting up a special session) is a feature that I would like to have. (With some physical devices I have serious trust issues 😉 and to have a small window where state/parameters are polled from time to time can help a lot)

So far I wrapped all async slots that qt sees only non-asyncio blocking functions. I didn't test qt-asyncio compatible event loops like asyncqt or quamash with tango etc. But changing the event loop within a running concert session (not a standalone python program) does not work.

@tfarago
Copy link
Contributor

tfarago commented Apr 6, 2022

Before we continue here this will require a discussion with the others, we could try to chat in the week after Easter.

@MarcusZuber
Copy link
Member Author

Sure. I just wanted to dump everything from my mind here, otherwise I can not remember until the next discussions 😉

I just played with it in the lab and it feels quite nice and responsive.

@tfarago
Copy link
Contributor

tfarago commented May 5, 2022

When I exit python bin/gui_standalone.py I get a segmentation fault 😨

self.read_button.clicked.connect(self.read)
self.write_button.clicked.connect(self.write)

# TODO: in standalone the run_in_loop is not working -> fix
Copy link
Contributor

@tfarago tfarago May 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be history after #482 if we manage to have the widget like this:

class ParameterWidget(QWidget, AsyncObject):
   def __ainit__(self, param):
        super().__init__()
        await super().__ainit__()

@MarcusZuber
Copy link
Member Author

When I exit python bin/gui_standalone.py I get a segmentation fault 😨

I'm quite satisfied in this state, that I only get this when exiting...

I already think to know the cause. When the full-async stuff is working I will fix it.

@codecov
Copy link

codecov bot commented Jun 22, 2022

Codecov Report

Patch coverage: 0.90% and project coverage change: -2.96 ⚠️

Comparison is base (4f2027c) 88.36% compared to head (ba5d6a3) 85.41%.

❗ Current head ba5d6a3 differs from pull request most recent head f0739e6. Consider uploading reports for the commit f0739e6 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #480      +/-   ##
==========================================
- Coverage   88.36%   85.41%   -2.96%     
==========================================
  Files         120      117       -3     
  Lines        8306     8151     -155     
==========================================
- Hits         7340     6962     -378     
- Misses        966     1189     +223     
Impacted Files Coverage Δ
concert/_ipython_setup.py 0.00% <0.00%> (ø)
concert/gui/__init__.py 0.00% <0.00%> (ø)
concert/gui/parameterizable.py 0.00% <0.00%> (ø)
concert/config.py 100.00% <100.00%> (ø)

... and 17 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@MarcusZuber
Copy link
Member Author

So far I'm quite happy with the functionality and interface. We should discuss how to proceed.

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

Successfully merging this pull request may close these issues.

2 participants