pcdshub / lightpath

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

ENH: better handling of disconnected PV's, add config functionality #155

Closed tangkong closed 1 year ago

tangkong commented 1 year ago

Description

Motivation and Context

closes #109 closes #152

It bears noting that lightpath doesn't necessarily care if devices are disconnected. It will represent them all the same. Currently a huge number of devices start off as disconnected, but become responsive by the time the GUI opens. This results in a lot of warning log messages spamming the terminal at startup. I decided to mute these messages since disconnected status should be obvious via the GUI.

How Has This Been Tested?

interactively, test has been added to the test suite

Where Has This Been Documented?

This PR. If we're lucky the docs will pick up the cli changes automatically...

tangkong commented 1 year ago

Moving the cli tests to the end of the test suite is a bandaid fix for now. It's possible there are problems more systemic in lightpath, but I haven't been able to pin them down further.

Essentially, the cli tests cause a segfault in later tests. If we only run one of the existing cli tests in the file (it doesn't matter which one), the suite will pass. Running the cli tests in isolation or at the end of the suite also passes. The even more strange part is that the segfault seems to arise based on the number of cli tests (duplicating any one of the tests will cause later gui tests to segfault).

A hand wavy explanation is that the cli tests create windows that don't get cleaned up properly by qtbot, and eventually qt or pytest runs out of memory.

I've chosen to move the tests to the end of this PR, but I'll leave a comment describing the danger at hand.

ZLLentz commented 1 year ago

Didn't see anything else when I reviewed the rest today- I'm :+1: after minor tweaks as above