philipstarkey
commented
6 years ago Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: philipstarkey).
I agree with the first half of this.
I don't think it solves labscript_devices pull request 27 though because (in my view) that data should be in connection_table_properties because it describes a fixed hardware configuration that cannot be configured at runtime and if it is different, it should invalidate the connection table (especially since the new kwarg in that pull request actually modifies the instruction set stored in the HDF5 file). What we need is a way to do backwards compatible checks on the connection_table_properties between different versions of labscript_devices that doesn't invalidate the connection table if the default behaviour of the new code matches the old code (and so the lack of a parameter is not important). I think I'll make a new issue for this so we don't clutter this one.
I'm ambivalent about changing the BLACS_connection column. We could do it, but the connection information would need to be stored in connection_table_properties with a consistent name (e.g. {"BLACS_connection":...}) in order for the forthcoming secondary control system/remote worker code to work (I'm assuming here that BLACS itself, rather than individual devices, needs to be able to determine which PC the BLACS tab/worker process are to be launched on and so BLACS needs to be able to find this information in a device agnostic way).
Original report (archived issue) by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).
Perhaps we should move device properties to the connection table dataset rather than attributes of device groups (with backward compatibility - property reading code can look for the device group if the column of the connection table doesn't exist). This would a) unify connection table properties and device properties, b) make the connection table dataset contain more of the information that is in a connection table .py file, c) allow saving device_properties on devices that do not have a BLACS tab.
We had a problem in the past where we added a device_property to all devices, and BLACS broke because it saw there was a device group for devices and tried to find a tab for them at shot run time and failed. Even though in that case the property was only really applicable for devices with BLACS tabs, you can imagine we might want to save more info for individual outputs, things that don't invalidate the connection table.
The device_properties column could be ignored by connection table code for the purposes of determining whether a connection table is a subset of/compatible with another.
Devices could still have free reign over their device group, but some of the things that are currently stored as attributes there could instead go in the device_properties column of the connection table group. A guide could be: if it's a user-set configuration, it goes in the connection table column, if it's programmatically generated by labscript compilation then it goes in the device group. To continue to take advantage of the serialisation and deserialisation in properties.py, we could rename the types of data that are saved in the device group from device_properties to something that emphasises that they are things programatically generated during compilation - like 'hardware_instruction_attributes'.
This would help things like labscript_devices pull request 27 not invalidate connection tables unnecessarily. That pull request could be modified to use device_properties already, but again, you can imagine wanting to separate user configuration and compilation results, as well as adding these properties for devices that do not have their own device group.
Another thing is BLACS_connection. This is superseded somewhat by the fact that devices can have many bits of data that tell BLACS how to connect to it, and these are increasingly going into connection_table_properties. I propose we get rid of the BLACS_connection column, replace it with a bool is_blacs_device or has_blacs_tab or similar - that way BLACS can still tell which devices it need to make tabs for - and move the actual connection information into connection_table_properties.
I think this can all be done in a backward compatible way.