labscript-suite / blacs

๐—ฏ๐—น๐—ฎ๐—ฐ๐˜€ supervises the execution of experiments controlled by the ๐˜ญ๐˜ข๐˜ฃ๐˜ด๐˜ค๐˜ณ๐˜ช๐˜ฑ๐˜ต ๐˜ด๐˜ถ๐˜ช๐˜ต๐˜ฆ. It manages experiment queuing and hardware-timed execution, and provides manual control over devices between shots.
http://labscriptsuite.org
Other
4 stars 48 forks source link

Blacs fails to close when using a PluginTab #86

Closed zakv closed 2 years ago

zakv commented 3 years ago

PluginTab seems to be missing some methods and attributes that blacs expects tabs to have. When using a class based on the example from here, everything works as expected until blacs is closed. At that point the error below is raised and blacs fails to exit. While failing to exit it keeps logging "INFO BLACS: destroy called".

Traceback (most recent call last):
  File "c:\...\qtutils\invoke_in_main.py", line 46, in event
    result = event.fn(*event.args, **event.kwargs)
  File "c:\...\blacs\__main__.py", line 605, in on_save_exit
    tab.shutdown_workers()
AttributeError: 'LockMonitorTab' object has no attribute 'shutdown_workers'

To get blacs to exit properly I had to add the methods shutdown_workers(self) and finalise_close_tab(self, *args, **kwargs), as well as add the attributes state and shutdown_workers_complete. Those methods don't need to do anything, but they need to be there to avoid the AttributeError and allow blacs to exit. Maybe the best solution is to just add these methods/attributes to PluginTab?

On a related note, the plugin tab doesn't have an icon and its text is black instead of blue, as expected from the link above.

chrisjbillington commented 3 years ago

Thanks for the report. It was always silly that the PluginTab duplicates code rather than inheriting or otherwise, hence this was missed when modifying the main tab class to shutdown worker processes more cleanly.

Probably the caller should just not call the shutdown_workers method unless it exists.

zakv commented 3 years ago

I briefly took a look at how hard it would be to change up the inheritance structure and it looks like it requires more knowledge of blacs internals than I have. I think your solution is pretty reasonable though and I'll probably get a chance to implement it in the next few days.