Skip to content

waitForTrigerr, from KbPressWait to KbCheck #116

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 5, 2020
Merged

waitForTrigerr, from KbPressWait to KbCheck #116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bc5ff23
change kbPressWait to KbCheck
marcobarilari Sep 30, 2020
a410e6f
modify the demo for best practice
marcobarilari Sep 30, 2020
6dc9542
delete the check for all key released
marcobarilari Sep 30, 2020
29254b2
fix md dead link
marcobarilari Sep 30, 2020
eb34ca3
Apply suggestions from code review
marcobarilari Oct 5, 2020
dbf563b
Update src/waitForTrigger.m
marcobarilari Oct 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ Tested:

## Documentation

All the documentation is accessible [here](./docs/00_index.md).
All the documentation is accessible [here](./docs/00-index.md).

## Content

Expand Down
14 changes: 6 additions & 8 deletions demos/CPP_waitForTriggerDemo.m
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
addpath(genpath(fullfile(pwd, '..', 'src')));
% add parent/src directory to the path (to make sure we can access the CPP_PTB functions)

%%
cfg.testingDevice = 'mri';
addpath(genpath(fullfile(pwd, '..', 'src')));

cfg.mri.triggerNb = 2;
cfg.mri.triggerNb = 5;

cfg.mri.triggerKey = 'space';
cfg.mri.triggerKey = 't';

KbName('UnifyKeyNames');

%%
% waitForTrigger(cfg);

%%
quietMode = true;
quietMode = false;
% waitForTrigger(cfg, [], quietMode);

%%
nbTriggersToWait = 1;
waitForTrigger(cfg, [], quietMode, nbTriggersToWait);
waitForTrigger(cfg, [], quietMode, cfg.mri.triggerNb);
6 changes: 3 additions & 3 deletions src/waitForTrigger.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ function waitForTrigger(varargin)
% triggers coming from the scanner in a real case scenario.
%
% INPUTS
% - varargin{1} = cfg
% - varargin{1} = cfg
%
% - varargin{2} = deviceNumber
%
% - varargin{3} = quietMode: a boolean to make sure nothing is printed on the screen or
% the prompt
%
% - nbTriggersToWait
% - nvarargin{4} = nbTriggersToWait

[cfg, nbTriggersToWait, deviceNumber, quietMode] = checkInputs(varargin);

Expand All @@ -43,7 +43,7 @@ function waitForTrigger(varargin)

keyCode = []; %#ok<NASGU>

[~, keyCode] = KbPressWait(deviceNumber);
[~, ~, keyCode] = KbCheck(deviceNumber);
Comment on lines 45 to +46
Copy link
Collaborator

@CerenB CerenB Oct 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @marcobarilari, I know you close this issue/merged this PR. But I want to ask two things.

  1. I'm using this function to get timestamp for my onsets by setting this line:
    [sec, keyCode] = KbPressWait(deviceNumber);
    and have the sec as output of this function.
    sec = waitForTrigger(varargin)

Then I call in my main exp script:
cfg.experimentStart = waitForTrigger(cfg)
Question here is do you see any logic mistake for storing onsets with this way of using the function?

  1. When I update my submodule with this PR, how can I "cherry pick" ? :D @Remi-Gau

Copy link
Collaborator Author

@marcobarilari marcobarilari Oct 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello!

(1) I don't think that there is any "logic mistake" in using such secondary measures unless they are not in the right place in a script. (primary one would be like startTime = PsychPortAudio(‘Start’, ...). It depends what is your purpose.

Though, I understand now why updating the submodels breaks your scripts :) and maybe here is the annoying thing that might tell you to not change function this way.

If I can suggest, you have the original waitForTrigger that, if I understand correctly, will pace your stimulation and right after that you can use

    
     waitForTrigger(cfg)

    onset = GetSecs;

    *stimulation*

    estStopTime = GetSecs;

    duration = estStopTime - onset;

Copy link
Collaborator

@CerenB CerenB Oct 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though, I understand now why updating the submodels breaks your scripts :) and maybe here is the annoying thing that might tell you to not change function this way.

look who is being mean now.. I only changed this function - only today. all the other submodule issues are due to submodule itself 👯

I'm using like this and it makes puurfect onsets (e.g. it's precisely right after my onsetDelay the stim starts) - without annoying decimal .00345 sort of things:

    % wait for trigger from fMRI
    cfg.experimentStart = waitForTrigger(cfg);
    ...
    % wait for baseline delays and then start the audio
    onset = PsychPortAudio('Start', cfg.audio.pahandle, [], ...
        cfg.experimentStart + cfg.timing.onsetDelay,1);

See my point? if I use GetSecs, then everything will be slightly weirdly numbered...

Also @marcobarilari I see that you do not use any buffer for your audio exp, so no audio cracks? That's good but I wonder you are buffering somewhere in your audio-mot localiser?

Copy link
Collaborator Author

@marcobarilari marcobarilari Oct 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using like this and it makes puurfect onsets

it is actually a fair point.....

I see that you do not use any buffer for your audio exp

we fill the buffer here

subfun/doAuditoryMotion.m line 46

% Start the sound presentation
PsychPortAudio('FillBuffer', cfg.audio.pahandle, sound);
PsychPortAudio('Start', cfg.audio.pahandle);
onset = GetSecs;

Copy link
Collaborator

@CerenB CerenB Oct 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup i saw that one and i was using that as well FillBuffer and Start... then in every first run, in the very first block there were audio cracks, so I'm using Start with option (see below). Then I remembered during my aud-mot exp, i was also inserting such option:

PsychPortAudio('Start', cfg.audio.pahandle, [], ...
        cfg.experimentStart + cfg.timing.onsetDelay,1)

Just to warn you to pay attention to audio ｡◕‿◕｡

btw, if you see my point,...does it mean you would consider changing the waitForTrigger for such optional use? ♥‿♥

Copy link
Collaborator Author

@marcobarilari marcobarilari Oct 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big dilemma :) can we pause this discussion for a couple of days? Maybe let me move this conversation in an issue and let's come back to it later.


if strcmp(KbName(keyCode), cfg.mri.triggerKey)

Expand Down