Skip to content

Conversation

@marcobarilari
Copy link
Collaborator

Changed KbPressWait to KbCheck so that triggerino can be used with this.

I did not add any check for all key released (the classic while KbCheck; end) because it is useless since the func has a pauseBetweenTriggers (either 0.5 secs or cfg.mri.repetitionTime / 2). With the check it has a weird behavior, I don't know why.

The 'downside' of that is that if you keep the triggerKey' pressed it will take the triggers at the pace of pauseBetweenTriggers`, but I think we can live with that.

fix #115

@marcobarilari marcobarilari added the priority 1 High priority label Sep 30, 2020
@marcobarilari marcobarilari linked an issue Sep 30, 2020 that may be closed by this pull request
@marcobarilari marcobarilari added priority 2 and removed priority 1 High priority labels Sep 30, 2020
@codecov
Copy link

codecov bot commented Sep 30, 2020

Codecov Report

Merging #116 into dev will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev     #116   +/-   ##
=======================================
  Coverage   29.05%   29.05%           
=======================================
  Files          47       47           
  Lines         702      702           
=======================================
  Hits          204      204           
  Misses        498      498           
Flag Coverage Δ
#unittests 29.05% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/waitForTrigger.m 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95f750e...dbf563b. Read the comment docs.

Copy link
Contributor

@Remi-Gau Remi-Gau left a comment

Choose a reason for hiding this comment

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

@marcobarilari
A tiny typo but then I am OK with merging this.

Co-authored-by: Remi Gau <remi_gau@hotmail.com>
@marcobarilari marcobarilari merged commit f3cd5b4 into cpp-lln-lab:dev Oct 5, 2020
@marcobarilari marcobarilari deleted the marco_waitForTrigger-update branch October 5, 2020 16:03
Comment on lines 45 to +46

[~, keyCode] = KbPressWait(deviceNumber);
[~, ~, keyCode] = KbCheck(deviceNumber);
Copy link
Collaborator

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

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

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

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

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

change waitForTriger to work with triggerino

3 participants