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

Happi Plugin Refactor Plan (UX) #549

Open
ZLLentz opened this issue Feb 23, 2023 · 2 comments
Open

Happi Plugin Refactor Plan (UX) #549

ZLLentz opened this issue Feb 23, 2023 · 2 comments

Comments

@ZLLentz
Copy link
Member

ZLLentz commented Feb 23, 2023

I'm creating this issue to document and gather feedback before I jump headfirst into a somewhat consequential refactor.

Background: Typhos plugin structure

  • The SignalPlugin in typhos.plugins.core uses the sig:// protocol and sends values and metadata from any ophyd signal, such as the non-epics signals. It is expecting the address to include the signal's name. Signals must be registered with the signal plugin. When a device is loaded into typhos via calling add_device on a display, all of its signals are registered. Note that this excludes devices sent in through the HappiPlugin.
  • The HappiPlugin in typhos.plugins.happi uses the happi:// protocol and sends a loaded happi device and its accompanying metadata to the tx_slot as defined in typhos.widgets.HappiChannel. It is expecting the address to either include the happi device's name or the happi device's attribute path to access a subdevice (or, any attribute of a subdevice). It sends a dictionary with keys "obj" and "md". Widgets need special handling to use the HappiPlugin, including a custom channel attribute that knows to use a HappiChannel. This is implemented in two places: first, on typhos.widgets.TyphosDesignerMixin, which is used for all typhos widgets except for the alarm widgets. The second place is on typhos.alarm.TyphosAlarm, which is used for the typhos alarm widgets. The variant on typhos.alarm.TyphosAlarm also allows other kinds of channels to be passed to use a normal epics connection or a signal as the object.
  • The HappiPlugin is not used except when the user invokes channel=happi://. Internally, when you launch a typhos device screen, it will load from happi without invoking the plugin, then call add_device.
  • In general, neither of these plugins are user-friendly and have a rough user experience (UX).
    • The SignalPlugin cannot be used outside of a typhos template without some setup in code, making it somewhat confusing to apply. On the flip side, it behaves very similar to the pydm built-in plugins so it's really nice to be able to re-use pydm widgets but with an ophyd signal. Users can get confused when there are two paths to accomplish the same goal: EPICS control points could be handled with the EPICS plugins instead, and there is no clear cost or benefit to either methodology. On balance, I don't think much needs to change here.
    • The HappiPlugin can be used anywhere, which makes it very convenient for embedding a typhos widget inside of a normal pydm display. UX-wise, however, it mimics the pydm channel interfaces while having completely different behavior, making it confusing to use. I would usually expect the channel field to be a source of live control data, but in this case it's a database source. Widgets that expect the HappiPlugin mostly ONLY accept the HappiPlugin, so you are required to pass happi:// even though all other options will fail. Users in general need to read the docs to figure out how to fill the widget regardless of their level of familiarity with pydm. Using it inside of a typhos template is clunky: you could use the ${name} macro to help access a subdevice, but this feels like a lot of boilerplate for something that should be a built-in. HappiPlugin widgets are impossible to fill with devices that aren't in happi via designer. I think this needs some adjustments.
  • When a typhos display loads, add_device is called on all typhos widgets. This is why you can place a PositionerWidget and leave the channel empty but still get a result when you open the screen.
    • There is currently a bug in this implementation where add_device is called regardless of existing devices or happi channel settings. This would be fixed by FIX: missing and extra device fills #548, but I paused that PR short of test suite updates to make this issue. I realized that in that PR it was awkward to test because there was no way to add a subdevice from a ui file without putting it into happi first.
    • Overall, this is nice. You don't need to do anything extra to fill device widgets with the screen's device.

Here is my plan:

SignalPlugin

  • The SignalPlugin will be extended slightly to ease the requirement of doing custom setup and make it easier to use.
  • If the user passes in a signal name with a "." in it, we will load a device via happi and use the given signal attribute as the data source in the widget. This will not require the signal to be registered.
  • If the signal plugin resolves to a non-signal, a helpful error message will be given.
  • The way that signals are automatically registered needs to be consolidated.

HappiPlugin

  • The HappiPlugin will stay and the channel field will also stay, but it will be consolidated into one implementation using the paradigm set in the alarm widgets: any non-happi protocol will create a signal that will be updated from the data source, and the signal will be sent as a "device" to add to the widget, which will need to handle this appropriately.
  • A standard designer field named happi_load_name will be added that will link directly to the HappiPlugin without considering the channel field.
  • A standard designer field named subdevice_attr will be added that will let us specify a subdevice to fill the widget with rather than the main device object
  • A standard designer field named device_source will be added, with options "automatic", "happi", "channel", and "explicit", which defaults to "automatic". This can be set manually or it will update automatically if exactly one of the related fields is filled. The typhos load process will call add_device if this is set to "automatic", will load from the HappiPlugin using happi_load_name if this is set to happi, will load from the channel address if this is set to "channel", and will create a new device if this is set to "explicit".
  • A standard designer field named explicit_load_string will be added that you can fill with e.g. ophyd.EpicsMotor("SOME:PREFIX", name="some_name") to include any device, even ones that aren't in happi.

The order of these fields will be:

  • device_source
  • happi_load_name
  • channel
  • explicit_load_string
  • subdevice_attr

I'd like to nail down the details here before proceeding: good idea? Bad? Misguided?

@klauer
Copy link
Contributor

klauer commented Feb 23, 2023

The background here is nicely written and comprehensive - thanks for the refresher.

As to the plan, I think it's good overall and agree with the path forward. As usual, I have my own opinions regarding potential implementation details. Let me see if I can convey what I'm thinking adequately...

I've mentioned before that I think the heavy reliance in PyDM on URIs like calc://a?b... with really long strings was an unfortunate or even bad design decision which is hostile to users. I think these attributes should contain structured data in a standard format (i.e., JSON) which can be more verbose and clear as to their intent. PyDM should bundle designer dialogs that help in modifying these in a sensible way.

I think a similar approach here could be beneficial (and would like to know what you think); it's an alternative to adding perhaps 5 or more fields to widgets that need to be handled separately in pyqt properties on widgets. In short, add a "device_source" field of type JSON that has a custom designer dialog that lets you easily search happi, select a component or subdevice or ..., or switch to a load string, etc. This should be well-formed JSON that corresponds with a dataclass that can be easily generated from Python as well, in case we want to programmatically generate them.

@ZLLentz
Copy link
Member Author

ZLLentz commented Feb 23, 2023

You're right, including custom designer dialogs is a step up in UX from "well-named fields with tooltips" and lets us work with structured data instead of limiting us to "the user gave a string" as in my proposal and in the pydm-style long URI formulations. When I dive into this, I'll start by exploring what can be done in these custom dialogs.

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

No branches or pull requests

2 participants