'Compile and restart' improvements - making sure it can happen when other things are borked

philipstarkey commented 9 years ago

Original report (archived issue) by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).

If there's anything wrong with the connection table, or really, if there is really a any of a wide range of failures during startup, the GUI should still pop up, the error displayed somewhere and the user have the possibility of (and be direct to consider) opening the 'compile and restart' dialog.

This option should be available even if BLACS has serious problems initialising, because some of these problems can be due to mismatches between connection table and BLACS tab code. For example, today I modified the Novatech's BLACS_connection to include the serial baud rate. I made it backward compatible (don't worry!), but before I did I had a connection table that didn't match what the BLACS tab expected. I couldn't start BLACS and I couldn't recompile the connection table except through runmanager. This has come up a lot in the wild if I understand correctly. So we need to just allow maximum breakage whilst still having the possibility of getting to that dialog.

Of course we should also do some basic checks for more obvious issues, like the connection table .py file doesn't exist (I think I want to copy a template to the given path in this case), or the h5 file doesn't exist (definitely want to bring up compile window in this case, probably with a message about 'do the first compile of this connection table').

We will also need the setting of what globals files to use to use in compilation to be available.

Is it much work to make compile_and_restart work standalone, but with an extra button to pop up the settings dialog for selecting globals? That button could be there even when the compile dialog is started normally from within BLACS - nothing wrong with having an extra way to get to the same dialog box.

I know it used to work standalone as a test. If we were to make it actually standalone we'd need it to know that it was standalone and not offer to restart BLACS (and look up the .py and .h5 files from labconfig, if it doesn't already do so).

Then we could add another shortcut to the labscript install folder, something like 'connection table compiler', that would just shortcut to python -m blacs.compile_and_restart --compile-standalone or something. We could direct users to use this to compile their connection table if things were broken.

In fact, if we'd rather not fix anything about BLACS startup, this alone would probably be satisfactory.

philipstarkey commented 9 years ago

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: pstarkey).

I don't think this will be too hard to do.

Firstly, blacs/compile_and_restart.py should be relocated to blacs/plugins/connection_table/ where the rest of the code lives for recompiling the connection table.

Then it should just be a matter of adding a main.py file to that dir, instantiating a labscriptutils.settings.Settings class, instantiating the plugin (which creates the settings page), and modifying the CompileAndRestart window to include a button to open a Settings window...

Actually, or second thought, it might be easier to just make BLACS start without needing a working connection table. We could show no tabs and a message suggesting recompilation if the connection table is missing of invalid. Would that achieve the same ends?

philipstarkey commented 9 years ago

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).

Yep, pretty much. I only suggested it be standalone as an addition, but having it always accessible so long as BLACS can start totally gets us what we want.

This would mean catching any errors that propagate from the connection table, even if the connection table itself looks fine when checked. Basically, all of BLACS' instantiation should be in a try: except:, and the error displayed in the GUI, maybe blanking out any tabs that did initialise correctly and displaying the error there.

philipstarkey commented 9 years ago

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: pstarkey).

So, question about desired behaviour.

If a Tab fails to load, do we wipe the saved front panel (and other) settings for that instance of the device or keep it around in the labconfig/_BLACS.h5 file indefinitely until the device loads correctly.

I'm in two minds about this. Sometimes front panel state is important to keep (eg flags need to stay high) and if a device tab fails to be created, you don't really want to throw out the front panel state and other settings just because you made a small typo in the DeviceTab class. Alternatively, if it is the saved settings preventing the tab from being created, you do want them forgetten!

Just thought of a potential solution as I was typing this:

BLACS starts, presents the user with an OutputBox which shows the log file as the rest of BLACS loads.
We load the FrontPanelSettings and instantiate the plugins. As long as it has got this far, the user can recompile the connection table
we then create the device tabs, and if any of these fail, None of the tabs are shown. The users is prompted to fix the problem with the code, or remove the offending device from their connection table. FrontPanelSettings are not saved on exit in this case (so no settings will be lost for devices that failed to instantiate), but the user is also presented with a button that will wipe FrontPanelSettings (for the failed devices) if they suspect that is the problem.

I probably need to make the QueueManager and AnalysisSubmission classes plugins to make this robust (as I need to be sure the current order I'm instantiating those objects with respect to the Plugins doesn't matter). This would move the ExperimentServer class to the QueueManager plugin, which is probably a wise separation of code anyway! It also will result in BLACS being far more modular (in that it will be easy for someone to replace the queuemanager/analysis submission) with a better/different version.

This will probably require a major version number bump as the interface for plugins may need to change in a backwards incompatible way.

philipstarkey commented 9 years ago

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: pstarkey).

Well if there are no objection to that solution, I'll go ahead and do it sometime soon

philipstarkey commented 9 years ago

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).

That sounds great!

If you want to make the QueueManager and AnalysisSubmission just the one plugin, so that they live together (if that is reasonable in the context of the plugin architecture), I won't complain. I have some ideas about how to make a more general, GUIless QueueManager and AnalysisSubmission class together, and would like at some point to write this to do everything that doesn't require a GUI or a connection table, and have BLACS subclass it and override methods to put GUI stuff and connection table stuff in where required.

I basically want to make a class that can be instantiated in a process by itself and have multiple clients pulling shots from it, and can respond to command line or zmq instructions to pause, repeat, etc. The thinking is that this would allow a drop-in replacement for BLACS for the purposes of simulations using the labscript suite (an idea multiple people have had that seems more excellent by the minute).

This seems approximately equally as hard whether it comes before or after the BLACS changes we're discussing here, and I haven't sufficiently fleshed out the details of how a bunch of things should work - but I'm fairly confident it's easier if the QueueManager and AnalysisSubmission functionality is tightly bound. What I think might make sense is for the AnalysisSubmission to remain its own class as it is now, but for the QueueManager to be a plugin and the QueueManager class to instantiate, own, and interact with an AnalysisSubmission instance. Nobody else needs to interact with it, so it should be hidden from them, and I think it makes sense for it to be considered a component of the Queue.

So that would be my only suggestion!

But you're more familiar with the plugin architecture (I am not familiar at all), so if you think it makes more sense for them to be separate, I'm happy with that too.