Skip to content

Comments

Centralize collection directory#299

Merged
JunAishima merged 22 commits intoNSLS2:2023-2-amxfmxfrom
JunAishima:centralize-collection-directory
Jun 27, 2023
Merged

Centralize collection directory#299
JunAishima merged 22 commits intoNSLS2:2023-2-amxfmxfrom
JunAishima:centralize-collection-directory

Conversation

@JunAishima
Copy link
Collaborator

  • Changes to ensure server and GUI are writing to the same directory
  • GUI
    • Not allowed to start if the directory where it is started is not the same as where the server is started
    • Will be terminated if the visit directory is changed while it is running
  • Server
    • Not allowed to start if the current_visit_lsdc_<tla> file does not exist or the CURRENT_VISIT_DIR variable does not point to a proposal directory-like name

JunAishima and others added 12 commits June 6, 2023 16:09
 * use this as the source of truth for both GUI and
   server
 * on startup. if not started there, show an error and
   exit
 * if it has changed, kill the GUI, and do this check
   in a separate thread
 * use QThread to allow ability to repeat
 * the GUI should terminate quickly after the visit directory is
   changed to prevent the user collecting data in an undesired place
 * user is not likely to have the GUI log open, so also ensure the
   message is printed out to the terminal
Added checks for server:
- Checks to if env file exists
- Checks if CURRENT_VISIT_DIR has a value and is valid
@JunAishima JunAishima force-pushed the centralize-collection-directory branch from e29e862 to 713237f Compare June 6, 2023 20:10
@JunAishima
Copy link
Collaborator Author

@vshekar - some information about the branches of LSDC in the current condition with the fixes from this pull request vs. what was running before

current branches containing the fixes:
AMX: 2023-2-amxfmx-production-dev
FMX: 2023-2-amxfmx-production-dev

previous branches that we must roll back to if necessary (pre-fix branches):
AMX: 2023-2-amxfmx-production (also archive-2023-05-16)
FMX: 2023-2-amxfmx-production-transmission-fixes

hutchTopCamThread.start()
serverCheckThread = ServerCheckThread(
parent=self, delay=SERVER_CHECK_DELAY)
serverCheckThread.visit_dir_changed.connect(self.close)
Copy link
Contributor

Choose a reason for hiding this comment

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

is app.exit (or similar) a better choice here? It does look like def closeEvent will exit, but directly exiting the app here may be more reliable?

Copy link
Contributor

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

I left a few comments that I think would reduce edge cases, but as-is this PR achieves the goals of:

  • only starting the server if it is in a directory with the correct structure
  • the GUI exits with in 2 seconds of the server directory being different than the cwd

JunAishima and others added 2 commits June 15, 2023 17:39
Co-authored-by: Thomas A Caswell <tcaswell@gmail.com>
JunAishima and others added 2 commits June 15, 2023 17:58
 * the object in question is a QLabel(), which is not editable
Inspired by:
```
if the goal is murder the app...just murder the app -Tom Caswell
```
@JunAishima
Copy link
Collaborator Author

JunAishima commented Jun 20, 2023

@tacaswell
Before: started at commit 02aca1 of LSDC, on 2023-2-amxfmx-production-dev, which includes up to commit 88b4ed of the centralize-collection-directory branch.

output with self.close():
The server visit directory has changed, stopping!

After: merged in latest centralize-collection-directory into 2023-2-amxfmx-production-dev. this includes the QApplication fix, in addition to top view fix for cwd. The latter was not checked.
output with QApplication.getInstance().quit:

The server visit directory has changed, stopping!
QPixmap::fromImage: QPixmap cannot be created without a QGuiApplication
QPixmap: Must construct a QGuiApplication before a QPixmap

The GUI application does stop so it still works.

@tacaswell
Copy link
Contributor

QPixmap::fromImage: QPixmap cannot be created without a QGuiApplication
QPixmap: Must construct a QGuiApplication before a QPixmap

That suggests that you have a slot someplace that does not check if the application is doing down before it does some work

@vshekar
Copy link
Collaborator

vshekar commented Jun 20, 2023

That is most likely from a QThread that is getting camera images in the background to put in the UI. I can add a fix to clean up these threads before the quit is called either in this PR or a separate one

- Using already existing closeAll() method
- Threads are stopped cleanly and quit
@JunAishima
Copy link
Collaborator Author

I'm still getting QPixmap: Must construct a QGuiApplication before a QPixmap error as the GUI is being killed. this is slightly different from before in that the first line of the previous error is no longer shown.

@JunAishima JunAishima merged commit 2334267 into NSLS2:2023-2-amxfmx Jun 27, 2023
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

Successfully merging this pull request may close these issues.

3 participants