search

pcdshub / typhos

Automatic-yet-customizable Graphical User Interface Generation for Ophyd Devices
http://pcdshub.github.io/typhos
Other
16 stars 24 forks source link

Initial value is set before enum choices in a PyDMEnumComboBox #605

Closed canismarko closed 1 week ago

canismarko commented 1 month ago

Expected Behavior

During initialization, a new pcdsdevices.plugins.core.SignalConnection object should set the enum_choices (really all metadata) first, then set the signal's value so that it can select from the available choices.

Current Behavior

When the PyDMEnumComboBox first loads, there is an error message from PyDM:

Can not change value to 'nA'. Not an option in PyDMComboBox

even though when I look at the widget, "nA" is one of the options. This happens regardless of what the enum_choices actually are.

I think this is because the SignalConnection.add_listener method first calls self.send_new_value(signal_val) then self.send_new_meta(**signal_meta).

After the widget is shown, If I change the signal's value outside of PyDM (e.g. though channel access), then the widget updates properly.

Possible Solution

Reversing the order of self.send_new_value(…) and self.send_new_meta(…) fixes this problem at my beamline.

I'm happy to prepare a PR that fixes this.

Steps to Reproduce (for bugs)

  1. Create a PyDM display with a PyDMEnumComboBox somewhere.
  2. Set the channel of the combo box to point to an Ophyd EpicsSignal obj with enum choices
  3. Load the PyDM display

Context

I have several windows that use this pattern. Right now, we get incorrect values shown in the PyDMEnumComboBox when it first loads. This makes it difficult to use, since we first have to wait for the PV to change some other way (or force it to change somehow). It'd be nice if it showed the correct value from the beginning.

Your Environment

Here are the list of installed packages I'm using: https://gist.github.com/canismarko/4f681511b295451179b421a14ba0566e

Running in a conda environment on a RHEL9 computer.

ZLLentz commented 1 month ago

Thank you for catching this, the explanation and the proposed fix both make sense to me. I'm happy to accept a PR or I'll make one myself in a bit (I happen to be working on typhos things this week)

ZLLentz commented 3 weeks ago

Sorry for the delay on getting back to these- I have a short vacation now and then next week these two tickets are reaching the top of my typhos todo list.

canismarko commented 2 weeks ago

No worries. I'd have submitted a PR by now myself except we're bringing our beamline back up right now after APS-U. Thanks.