Event buttons and task API

[alustig3]

Event buttons

Right now you can update task variables from the GUI. The ability to manually send events from the GUI would also be useful (feature requested here)

This pull request adds a special button variable. When a user adds btn_examplebutton = "event_name" to their task, the Controls dialog will automatically create a button, that when clicked, will generate that event within the task. See example/button.py

Task API

An API for could be quite useful for...

• offloading intensive computations onto the PC i.e. run statistics on large arrays using scipy, then send result the result back to the pyboard to be used within the task.
• making custom live plots using matplotlib
• making external calls to other external programs such as psychopy (feature requested here).

Huge thanks to @neuromantic99! All of the hard work was done on his api_dev branch

some minor changes I made/didn't include:

• allow API to add events to event queue
• added "vars" key to API data so users can execute code based on variable changes
• did not include code (in api.py or run_experiment.py) for communication between APIs when multiple setups were running during an experiment

Summary of changes to framework

• GUI can now generate events
• User script now has access to task through API
  • can receive data
  • can set variables
  • can generate events
  • can make calls to external libraries/programs

Before:

After:

@ThomasAkam one concern is that the added user_classes could potentially effect task logic but is not included in the data file or accounted for in the versioned task hash. This could harm reproducibility. Hopefully this versioning issue for user_classes and hardware definitions https://github.com/pyControl/code/issues/52 can be addressed with the new data file format you are working on?

[ThomasAkam]

Many thanks @alustig3, this is really fantastic, both bits of functionallity will be super useful. A couple of comments, questions and suggestions:

Implementation

• For the GUI event triggering functionallity, rather than having users specify which events they want to be able to trigger in advance in the task file, I would be inclined to have a row in the 'Controls' dialog which has a lablel saying 'Trigger event:' then a dropdown menu to select which event, and a button to trigger it. This seems both simpler for the user and more flexible. What do you think?
• We will ideally want to indicate which events are triggered by the GUI in the log file, but this can wait till the new data format is merged.
• I would be inclined to have one or more dedicated task to show the API functionallity, as the blinker and button examples are the simplest tasks and are likely a starting point for users familiarising themselves with task coding, so I would rather not complicate them.

Code

• Pycboard.send_serial_data() : Could header and subheader have more meaningful names? Or be combined into a single header argument?

• Pycboard.generate_event() : Would trigger_event be a clearer function name? 'event_name' might be clearer than 'event' for the argument to differentiate from the event ID.

• Api :

  • How is the Api.update() call frequency determined, this could be stated in the update docstring.
  • The various named tuples (e.g. event_tup) could be defined at the module level rather than inside API.interface().

• framework.py: Why move the code in the read_data function out from the recieve_data function. If the code is not used elsewhere there is no reason to have a seperate function, and function calls incur some amount of overhead.

[ThomasAkam]

@alustig3 I think the new_data_format branch is ready to merge back into the dev branch. Do you want me to wait till this is merged in or shall I go ahead and do that now?

[alustig3]

@ThomasAkam please go ahead and merge the new_data_format branch

[ThomasAkam]

I've merged the new_data_format branch into dev. I've also formatted the dev branch with the Black formatter using a max line length of 120 as discussed in #82.

[alustig3]

For the GUI event triggering functionallity, rather than having users specify which events they want to be able to trigger in advance in the task file, I would be inclined to have a row in the 'Controls' dialog which has a lablel saying 'Trigger event:' then a dropdown menu to select which event, and a button to trigger it. This seems both simpler for the user and more flexible. What do you think?

Yes that is a great solution. I like that it avoids the need for another special variable type. I made the changes as you described for the standard control dialog. Individual custom event triggering buttons can still be created using the custom controls dialog editor.

I would be inclined to have one or more dedicated task to show the API functionallity, as the blinker and button examples are the simplest tasks and are likely a starting point for users familiarising themselves with task coding, so I would rather not complicate them.

I reverted the changes to the example tasks and created a separate (rough) example api task (please change as you see fit).

Pycboard.send_serial_data() : Could header and subheader have more meaningful names? Or be combined into a single header argument?

changed to command and cmd_type

Pycboard.generate_event() : Would trigger_event be a clearer function name? 'event_name' might be clearer than 'event' for the argument to differentiate from the event ID.

yes changes made

How is the Api.update() call frequency determined, this could be stated in the update docstring.

based on plotting update interval setting. I believe it was added for the purposed of being used with matplotlib or equivalent. I gave it a clearer function name and updated the docstring.

The various named tuples (e.g. event_tup) could be defined at the module level rather than inside API.interface().

I believe it was written this way because if the tuples are in the API __init__function, they could potentially be left out when the user writes their sub_class(API) if they end up defining their own __init__ function and forget to include super().__init__(). I'm not sure though? Also I might be misunderstanding what you are asking about.

framework.py: Why move the code in the read_data function out from the recieve_data function. If the code is not used elsewhere there is no reason to have a seperate function, and function calls incur some amount of overhead.

fixed

@ThomasAkam I think for the most part this is ready to merge, and then you can make the final fixes to get this across the finish line?

• I was not able to test the API with analog data so please test if you can
• like you said above, it'd be ideal to indicate which events are triggered by the GUI in the log file
  • by the same token, right now the user_class can self.print_to_log(). should it instead create a print_type event so it get's logged to the data file?

[ThomasAkam]

@alustig3 Many thanks for making these changes, I'll merge this into the dev branch now.

I noticed there are quite a lot of formatting changes in bits of the code unrelated to the new functionallity. Please do these in seperate pull request in future as it makes reviewing the pull requests harder.

Also, regarding the reformatting of comments, for comments that are at the end of a line and indicate the meaning of a variable, I think that moving them to the line above is confusing as they can be taken for a section heading. I would rather move them to the line below and indent them, or leave the variable definion split across multiple lines with brackets and keep the comment at the end of the line defining the variable.

[alustig3]

Please do these in seperate pull request in future as it makes reviewing the pull requests harder.

will do

I would rather move them to the line below and indent them, or leave the variable definion split across multiple lines with brackets and keep the comment at the end of the line defining the variable.

sounds good

thanks @ThomasAkam for cleaning things up and doing the merge.

[alustig3]

I've attempted to add event types for calls made by the API: https://github.com/alustig3/pyControl/commit/51f5919ec451fbe92a65f418f42a147c43ae3b7c

I don't know if it is the right approach, and I'm not sure how to proceed with giving API triggered events its own type since unlike varbl_typ, the event_typ doesn't have subtypes that I could add to.

@ThomasAkam could you take a look at implementing this?

Also, is there a way to send a print_typ event from the API? Right now the API class is written to use a print_to_log function that prints directly to the GUI log without a timestamp and without getting written to the data log.

[ThomasAkam]

Hi @alustig3,

If I understand correctly the new code that you linked to is for recording whether a set_variable is due to the api or the user, but does not implement indicating whether events are triggered by the api, is that correct?

I can see that it would be desirable to indicate when events are api triggered, and I think the way to do this would be to have a single event_typ which had an additional field in the event tuple which indicated whether it was triggered by a timer, external input or API. This could be recorded in the value column of the data tsv. Alternatively we could have an additional column called e.g. 'operation' which would indicate the operation that generated the row, so {'timer', 'input', 'api'} for events and {'run_start', 'run_end', 'user_set', 'api_set', 'get', 'print'} for variables. This might be preferable as it is already a bit ugly to use the 'name' column for the operation that generated a 'variable' row and it would get even worse if we then use the 'value' column to indicate the operation that generated an event row. What do you think?

[alustig3]

@ThomasAkam,

If I understand correctly the new code that you linked to is for recording whether a set_variable is due to the api or the user, but does not implement indicating whether events are triggered by the api, is that correct?

correct

the following is my understanding: the dev branch currently has

time	type	name/ID	value
timestamp	state	state name	-
timestamp	event	event name	-
timestamp	print	-	print text
timestamp	variable	get/set/print/run_start/run_end	{"var":value,"var2":value2}
-	warning	-	warning message text
-	error	-	error message text

option 1:

have a single event_typ which had an additional field in the event tuple which indicated whether it was triggered by a timer, external input or API. This could be recorded in the value column of the data tsv

time	type	name/ID	value
timestamp	state	state name	-
timestamp	event	event name	timer/ext_input/api
timestamp	print	-	print text
timestamp	variable	get/user_set/api_set/print/run_start/run_end	{"var":value,"var2":value2}
-	warning	-	warning message text
-	error	-	error message text

option 2:

Alternatively we could have an additional column called e.g. 'operation' which would indicate the operation that generated the row, so {'timer', 'input', 'api'} for events and {'run_start', 'run_end', 'user_set', 'api_set', 'get', 'print'} for variables

time	type	name	operation	value
timestamp	state	state name	-	-
timestamp	event	event name	timer/ext_input/api	-
timestamp	print	-	-	print text
timestamp	variable	-	get/user_set/api_set/print/run_start/run_end	{"var":value,"var2":value2}
-	warning	-	-	warning message text
-	error	-	-	error message text

I agree that option 1 is a bit ugly/inconsistent. I propose a variation of option 2, but combine the name/value columns.

time	type	operation	name/value
timestamp	state	-	state name
timestamp	event	timer/ext_input/user/api	event name
timestamp	print	task/api	print text
timestamp	variable	get/user_set/api_set/print/run_start/run_end	{"var":value,"var2":value2}
-	warning	-	warning message text
-	error	-	error message text

what do you think?

[ThomasAkam]

I agree it would be desirable to stick with 4 columns given we don't need 5, but 'operation' and 'name/value' might be confusing for info rows. How about 'subtype' and 'content'.

time	type	subtype	content
timestamp	info	info_type	info_value
timestamp	state	-	state_name
timestamp	event	timer/input/user/api	event_name
timestamp	print	task/api	print_text
timestamp	variable	get/user_set/api_set/print/run_start/run_end	{"var":value,"var2":value2}
-	warning	-	warning message text
-	error	-	error message text

[alustig3]

looks good to me 👍