cpp-lln-lab / CPP_PTB

a set of function to make it easier to create behavioral, EEG, fMRI experiment with psychtoolbox
https://cpp-ptb.readthedocs.io/en/latest/index.html#
MIT License
12 stars 13 forks source link

Refactor getResponses #27

Closed Remi-Gau closed 3 years ago

Remi-Gau commented 3 years ago

A lot of refactoring and @CerenB this should also fix #19 and problem of extra responses you were getting from getResponses.

Remi-Gau commented 3 years ago

Added a checkAbort function that checks that the the cfg.escapeKey has been pressed and throws an error to stop everything if that's the case.

This function is also called every time getResponse('check') is triggered.

@CerenB this should fix #22

Remi-Gau commented 3 years ago

TO DO @marcobarilari once we have merged this one https://github.com/marcobarilari/CPP_PTB/pull/2 and #26 I will rebase and add cfg.escapeKey as one of the default value for initPTB.

Remi-Gau commented 3 years ago

function checkAbort only would work if in the main script I'm using for loop, right? It'd break the for loop with stopEverything. What if a user is not using any for loop in their experiment?

Actually checkAbort is pretty "aggressive" at the moment and will just throw an error so it will stop everything no matter what (even with no loop).

I trying to think of a way to make it abort more "gracefully" (like give a chance to save stuff) and that's why I wanted to create a global variable.

Or do you think this is OK?

CerenB commented 3 years ago

well I was thinking that warning instead of error would be nicer. Unfortunately I could not make this abort work even with the error, even with a long press.

Remi-Gau commented 3 years ago

I don't think this should be related to stopEverything.

I have added a demo that should work and is very similar to what you sent.

Remi-Gau commented 3 years ago

If this does not work try setting the cfg.escapeKey to space and check if this works when you press the space bar.

At least we will know this is likely not "key" related.

Remi-Gau commented 3 years ago

Been thinking of how to implement this "abort" as part of getResponse.

At the moment getResponse only checks the response box, not the keyboard of the experimenter. So the only way to make this "abort" thing work woud be to ask getResponse to check for keypresses on all devices.

I could make this the default and listening to the responseBox would have to be asked for.

CerenB commented 3 years ago

that's one of points I got stuck with in getResponse function. I thought now it's listening to all key presses. OR is it? It's listening to responseBox specific button presses, but in keyboard all presses? Maybe when I was checking on Friday, I got stuck at this point because I specified the keys to check but it was checking all the key presses (no additional keyboards were connected than built in mac keyboard)

CerenB commented 3 years ago

to respond to your question, how about separate these two functions completely? Both uses KbQueue checks/buffering. Aborting function checks keyboard and aborts things - maybe there's a way to do it smoothly when things are independent two functions?

marcobarilari commented 3 years ago

function checkAbort only would work if in the main script I'm using for loop, right? It'd break the for loop with stopEverything. What if a user is not using any for loop in their experiment?

Actually checkAbort is pretty "aggressive" at the moment and will just throw an error so it will stop everything no matter what (even with no loop).

I trying to think of a way to make it abort more "gracefully" (like give a chance to save stuff) and that's why I wanted to create a global variable.

Or do you think this is OK?

Maybe, since it prompts an error, we can have something in the catch part that will save everything (workspace, button presses in the buffer and close the file, eye tracker data, etc).

Does it make sense?

Remi-Gau commented 3 years ago

to respond to your question, how about separate these two functions completely? Both uses KbQueue checks/buffering. Aborting function checks keyboard and aborts things - maybe there's a way to do it smoothly when things are independent two functions?

Need to check if we can have to KbQueues going in parallel.

Remi-Gau commented 3 years ago

that's one of points I got stuck with in getResponse function. I thought now it's listening to all key presses. OR is it? It's listening to responseBox specific button presses, but in keyboard all presses? Maybe when I was checking on Friday, I got stuck at this point because I specified the keys to check but it was checking all the key presses (no additional keyboards were connected than built in mac keyboard)

So by default getResponse listens to the responseBox but if you have only one device then the keyboard and the responseBox should be the same.

Did you specify the keys to check like this?

expParameters.responseKey = {'a', 'b', 'c'}; 
Remi-Gau commented 3 years ago

OK what I am thinking of starting to do is to have getResponse and the abort function to check for all devices by default (to make it most general) and then implement a way to let the user decide which device (keyboard or response box), each function should listen to.

This will require some changes to the default and the initPTB I think but nothing crazy.

This might make things less confusing for most new PTB users so that's not bad.

Remi-Gau commented 3 years ago

OK so a series of changes on all the keyboard functions:

Remi-Gau commented 3 years ago

Similarly will merge in 24 hours unless I get a veto. :smile: