Hardware specific variables

[alustig3]

this addresses #20

CleanShot.2023-03-29.at.00.40.51.mp4

[ThomasAkam]

Many thanks for implementing this, I think this will be very useful but have one high level question about the implementation. Currently if I understand correctly the hardware level variables are still associated with individual tasks. It seems to me that it might be preferable if the hardware level variables are independent from individual tasks, as the type of things they are likely to be useful for, such as solenoid or other hardware calibration, will be used across many tasks on the same setup. One way this could be implemented would be to have hardware level variables defined in the hardware definition. Annother way would be to have the hardware level variables not associated with any task in the GUI, but if a corresponding variable is defined in any task used on the setup the variable will be set. This could be combined with a convention such that hardware variable names always start with hw, e.g. v.hw_my_variable. What do you think?

[alustig3]

CleanShot.2023-04-07.at.15.19.43.mp4

I changed it so that hardware level variables are independent from individual tasks. I didn't want to have them defined in hardware definitions because that would mean many individual and nearly identical hardware definition files would need to be created and tediously reloaded every time there was a change to a setup. I went with your suggestion of having a special v.hw_my_variable within a task.

• When specifying the hardware variable, you put "my_variable" and the value. no need to put "hw_" at the start of the variable.
• currently if the hardware variable is not specified in the setups.json, then the task will fall back to whatever the default value in the task is. I'm not sure if this is the ideal behavior. maybe it should throw an error and stop the task upload? At one point I had a QMessagebox warning appear in this situation, but it only worked for run_task. In run_experiment, the QMessagebox broke things in the parallel_call.
• Also, the v.hw_my_variable is currently editable in the variables dialog. Maybe it should automatically be hidden, like how v.custom_dialog and variables that end in "___" currently are? If not, then how should we deal with a hardware variable being marked as a persistent variable? I guess the persistent value would take priority? Should the persistent variable update the setups.json at the end of a session?

[alustig3]

Also, the v.hw_my_variable is currently editable in the variables dialog. Maybe it should automatically be hidden, like how v.custom_dialog and variables that end in "___" currently are? If not, then how should we deal with a hardware variable being marked as a persistent variable? I guess the persistent value would take priority? Should the persistent variable update the setups.json at the end of a session?

I changed it so v.hw_variables are hidden from the task and experiment variable GUI elements. I think this makes sense, and is simplest.

[ThomasAkam]

Thanks for the updates and apogies for the delay getting back to this.

I changed it so that hardware level variables are independent from individual tasks. I didn't want to have them defined in hardware definitions because that would mean many individual and nearly identical hardware definition files would need to be created and tediously reloaded every time there was a change to a setup.

I'm not sure I understand this concern as I don't see why having variables in the hardware definition would require duplicaton of hardware definitions beyond that needed to deal with different hardware setups?

I went with your suggestion of having a special v.hw_my_variable within a task.

I think this is a reasonable way to go but the one thing I am not crazy about is that because the user has to enter the variable name manually in the variables table, there is a risk of a typo in the variable table or task file causing the variable not to be set, which it would be easy to miss when the task is run on systems with large numbers of setups. I agree that giving a warning or error if a hardware variable defined in a task is not set would be desirable to avoid this. For the Experiment tab it might be best to do this in the Configure_experiment_tab when the experiment is run, rather than in the Run_experiment_tab, as this would avoid issues with the parallel_call and also quicker for the user.

When specifying the hardware variable, you put "my_variable" and the value. no need to put "hw_" at the start of the variable

This might be confusing for users as it is different behaviour from the standard variables table in the run experiment tab. I think my preference is to use the full name.

I changed it so v.hw_variables are hidden from the task and experiment variable GUI elements. I think this makes sense, and is simplest.

Agree this is sensible.

• Do you think it would be preferable to have a single hardware variables table where you set variables for all the setups, rather than seperate tables for each setup? In situations where you e.g. set calibration data for a number of setups at the same time this might be more convinient, but I don't feel strongly about it and the current implementation is much better than no implementation!

[alustig3]

I am not crazy about is that because the user has to enter the variable name manually in the variables table, there is a risk of a typo in the variable table or task file causing the variable not to be set, which it would be easy to miss when the task is run on systems with large numbers of setups

The user now selects the variable name from a dropdown instead of typing it manually. All task files are automatically scanned for hardware variables and put into a list that is used to populate the dropdown. If you want to add/edit hardware variables, you need to first add it to a task file.

I agree that giving a warning or error if a hardware variable defined in a task is not set would be desirable to avoid this. For the Experiment tab it might be best to do this in the Configure_experiment_tab when the experiment is run, rather than in the Run_experiment_tab, as this would avoid issues with the parallel_call and also quicker for the user.

A warning message box now pops up if a task has a hardware variable defined but the setup that is trying to run does not have a corresponding value for that hardware variable. To avoid parallel_call issues, this check/warning happens in the configure_experiment_tab as you suggested.

I think my preference is to use the full name.

fixed

Do you think it would be preferable to have a single hardware variables table where you set variables for all the setups, rather than separate tables for each setup

I agree this is preferable. You can now set variables for selected setups. I added a "Variables" button to the "Configure selected" section.

These changes are fresh and have not undergone a lot of testing, so please let me know if you find any bugs.

[alustig3]

@ThomasAkam this is ready for review when you get a chance

[ThomasAkam]

Thanks Andy, these changes sound great but currently do not seem to be working on my machine - I can't select any setups in the setups tab using either the 'select all' checkbox or the individual setup checkboxes. No error message is generated when I click the checkboxes.

pyControl.v1.8.2023-05-03.10-23-36.mp4

[alustig3]

should be fixed now @ThomasAkam

[ThomasAkam]

Thanks @alustig3, functionallity appears to be working great appart from a couple of small bugs:

• Pressing the setups tab variables button before naming any setups gave the following error:

2023-05-04 17:40:45,420 Traceback (most recent call last):
  File "E:\Dropbox\Hardware development\pyControl contributor repos\pyControl_al3\gui\setups_tab.py", line 218, in edit_hardware_vars
    hardware_var_editor = Hardware_variables_editor(self)
  File "E:\Dropbox\Hardware development\pyControl contributor repos\pyControl_al3\gui\hardware_variables_dialog.py", line 48, in __init__
    self.update_var_table()
  File "E:\Dropbox\Hardware development\pyControl contributor repos\pyControl_al3\gui\hardware_variables_dialog.py", line 51, in update_var_table
    self.var_table.fill_table(self.variable_cbox.currentText())
  File "E:\Dropbox\Hardware development\pyControl contributor repos\pyControl_al3\gui\hardware_variables_dialog.py", line 97, in fill_table
    with open(self.setup_var_editor.setups_tab.save_path, "r", encoding="utf-8") as f:
FileNotFoundError: [Errno 2] No such file or directory: 'E:\\Dropbox\\Hardware development\\pyControl contributor repos\\pyControl_al3\\config\\setups.json'

-Pressing the setups tab variables button after removing the name of a setup gave this error:

2023-05-04 17:51:44,454 Traceback (most recent call last):
  File "E:\Dropbox\Hardware development\pyControl contributor repos\pyControl_al3\gui\setups_tab.py", line 218, in edit_hardware_vars
    hardware_var_editor = Hardware_variables_editor(self)
  File "E:\Dropbox\Hardware development\pyControl contributor repos\pyControl_al3\gui\hardware_variables_dialog.py", line 48, in __init__
    self.update_var_table()
  File "E:\Dropbox\Hardware development\pyControl contributor repos\pyControl_al3\gui\hardware_variables_dialog.py", line 51, in update_var_table
    self.var_table.fill_table(self.variable_cbox.currentText())
  File "E:\Dropbox\Hardware development\pyControl contributor repos\pyControl_al3\gui\hardware_variables_dialog.py", line 103, in fill_table
    if setups_json[setup.port].get("variables"):
KeyError: 'COM4'

I guess these errors could be prevented by greying out the variables button unless all setups are named.

• Running the hardware_setup_level_variable.py example task gave an error due to the use of an f-string in the task file which is only supported on recent micropython versions. This was fixed by changing the task to use "".format().

[alustig3]

@ThomasAkam should all be fixed now. Thanks for testing!

[alustig3]

the variables button should be greyed out unless 1 or more of the selected setups has a name. Once the variables button is clicked, the dialog will only add to the editing table the subset of the selected setups that have names.

[ThomasAkam]

Thanks @alustig3 . I think we should aim to do a release soon to get all this new functionallity out. I would like to include the new_data_file_format in the next version, and will try and get that branch ready to merge ASAP. Are there any other bits of new functionallity you want to include before a new release?

[alustig3]

sounds good @ThomasAkam. There isn't any functionality that I'd like to add before the next release.

I would like to run the Black formatter (as discussed previously #67 (comment)) on all of the python files in the repository though.

It will probably be easiest if I make a quick pull request with formatting changes after you have merged the new_data_file_format into the dev branch. I can also add some information about using Black to the contributing section of the pyControl docs.