Skip to content

Conversation

@Remi-Gau
Copy link
Contributor

very few changes in the repo itself but a lot actually happened in the CPP_PTB submodule.

@marco can I let you test it to make sure it runs fine?

I still need to refactor the setParameters to have all the stuff related to the MT / MST will be in the same spot.

This was linked to issues Sep 25, 2020
@codecov
Copy link

codecov bot commented Sep 25, 2020

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##             dev     #59   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files         11      11           
  Lines        169     173    +4     
=====================================
- Misses       169     173    +4     
Flag Coverage Δ
#unittests 0.00% <0.00%> (ø)

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

Impacted Files Coverage Δ
subfun/doDotMo.m 0.00% <0.00%> (ø)
subfun/preTrialSetup.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 9454d50...f16bcff. Read the comment docs.

@marcobarilari
Copy link
Collaborator

Run 1 time and giving here the first impression:

  • very nice 👍

  • the red cross stays up for a long time

  • in the command window I don't see printed the events happening

TBC

Copy link
Collaborator

@marcobarilari marcobarilari left a comment

Choose a reason for hiding this comment

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

Now our localizer can do a lot of stuff and it is great, docs will need to explain all its different capabilities and how to implement them.

Let me know if I can help refactoring the setParameters func, after that I think it's ready

@Remi-Gau
Copy link
Contributor Author

  • compare to old version from Moh's

@marcobarilari marcobarilari added the priority 1 High priority label Sep 30, 2020
@Remi-Gau
Copy link
Contributor Author

Remi-Gau commented Oct 1, 2020

fixes #64

@marcobarilari
Copy link
Collaborator

marcobarilari commented Oct 2, 2020

LGTM!

Only one last thing, unless I missed something in the MRI we will present the MT loc on the whole screen so we don't care to 'rescale' to the size of the actual subject view on the mirror.

Will we need to tweak something to present the MT_MTS loc in a smaller central portion of the screen?

(if changes are needed we can do it in a different PR, if I remember well initPTB can handle this thing and I can take care of it)

@Remi-Gau
Copy link
Contributor Author

Remi-Gau commented Oct 2, 2020

LGTM!

Only one last thing, unless I missed something in the MRI we will present the MT loc on the whole screen so we don't care to 'rescale' to the size of the actual subject view on the mirror.

Will we need to tweak something to present the MT_MTS loc in a smaller central portion of the screen?

(if changes are needed we can do it in a different PR, if I remember well initPTB can handle this thing and I can take care of it)

Yes let's do that in another PR. I need to get this one off my chest. :-)

@marcobarilari
Copy link
Collaborator

ready to be merged

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

Labels

priority 1 High priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

implement radial motion Make script for MT/MST

2 participants