search

pcdshub / lightpath

LCLS Lightpath Module
https://pcdshub.github.io/lightpath
Other
4 stars 9 forks source link

Follow-up bug fixes, gui details #142

Closed tangkong closed 1 year ago

tangkong commented 1 year ago

Description

Fixes a slew of issues/errors found while running the gui on the updated happi db. Individual problems will be documented below.

Detailed problem documentation ### 1. EPICS Context unset on teardown - **Problem**: on teardown, an immense amount of traceback initiated by EPICS CA contexts being unset. - **Explanation**: On shutdown, nothing is done to gracefully complete/close any in-progress callbacks. - **Fix**: Hooked the closeEvent method to run a clean-up method that destroys all the `lightpath_summary` signals before closing. (This might be a bit heavy handed, but it worked) ### 2. GUI subscriptions should subscribe to `lightpath_summary` signal - **Problem**: widget subscriptions still subscribed to the whole device with SUB_STATE. This resulted in failures of the widget to subscribe to some devices, due to SUB_STATE being missing. - **Fix**: subscribe to `lightpath_summary` directly in the LightRow widget. Remember to initialize the `SummarySignal` by calling `lightpath_summary.get()` after subscribing ### 2.1. Getting stuck waiting on a Lock forever while calling `lightpath_summary.get()` - **Problem**: After changing the subscription method in LightRow, GUI initialization would hang. - **Explanation**: This was tracked down to an issue with locks colliding. Signal callbacks subscribed by the GUI will run while the `lightpath_summary._lock` is claimed/locked. These callbacks also had a chance to try and subscribe widgets to devices, which in turn called `lightpath_summary.get()` and attempted to grab `lightpath_summary._lock`. - ~~**easy fix**: keep track of if `lightpath_summary.get()` has been called, and only call it once~~ - **better fix**: don't call `lightpath_summary.get()` at all, since value callbacks will initialize and run callbacks on their own. Previously we needed to do this for the test suite to run, but switching from AttributeSignal fixed this in the test suite ### 3. `get_lightpath_state` returns None - **Problem**: occasionally a device would fail to return a `LightpathState` from `get_lightpath_state`, despite devices working in isolation - **Fix**: Currently just sleeping after LightController creation and before LightApp creation quenches these errors. Should probably do something more robust. ### 4. TimoutErrors, pyepics failed to send connection and access rights... - **Problem**: many PV's fail to connect within the default 1.0s. And with the huge number of them we can get a lot of errors - **Fix**: Increase the default timeout settings at the `EpicsSignalBase` level ### 5. `get_lightpath_state()` returns None, and subsequent methods fail - **Problem**: At startup, many instances of `Unable to determine device state`. The GUI will open, with many devices in an indeterminate (purple) state. Eventually callbacks will refresh the GUI, and the beampath will be updated. - **Fix**: Can sleep at startup to let PV's connect and allow devices to fully initialize. This is probably bad. This is also not the most breaking issue, it's more annoying than anything. ### 6. Beamlines that end in the middle of a branch - **Problem**: #137 . Some beamlines end in the middle of a branch, and there was no handling of this before - **Fix**: allow the config to be a dictionary of {branch_name: `end_z`} mappings, and split paths through that branch on the given `end_z`. ### 7. Upstream checkbox does nothing - **Fix**: Change behavior to allow user to select a device to filter devices upstream of. Now that devices live on branches rather than beamlines, there is no natural way to determine what a-priori a device to consider the first on the beamline. image

Tests and fixtures were also touched up, notably to properly prevent devices from being cached by happi, which led to callbacks being carried over between tests. Despite the short description this one brought me pain.

Motivation and Context

I want to actually run the lightpath app.

How Has This Been Tested?

test suite, and lots of interactive testing.

Where Has This Been Documented?

This PR, for now

tangkong commented 1 year ago

Not the longest lines-of-code-wise, but there are enough separate ideas/issues addressed here that I'd like to close the loop on this before moving on to other issues. The LCLS lightpath now opens and updates for all the beamlines, though it keeps a few non-breaking errors/output spam.