MRG, ENH: Check resolution and improve sync_test

[larsoner]

Adding:

"SCREEN_SIZE_PIX": "1920,1200",

now ensures that the fullscreen window obtained is this resolution. If I set it to something else (e.g., 800,800) I get an error message:

RuntimeError: Requested window size [800 600] but got size (1920, 1200), is the resolution set incorrectly?

Also sync_test.py now has a rectangle that is the standard size of a credit card, which hopefully makes it fairly easy to check.(Also you should be able to see if the credit card "looks funny", though you maybe probably see that with the circle.) I've checked and on my screen with proper expyfun.json it is indeed the size of a credit card.

@nordme @drammock can you review the code and try it out at least sync_test.py on your system(s)?

Closes #404

[drammock]

seems like the error message lists the resolutions backwards, as to which was requested vs. got?

[larsoner]

It looks correct to me at least? Where is it wrong?

[codecov[bot]]

Codecov Report

Merging #405 into master will decrease coverage by 0.59%. The diff coverage is 96.29%.

@@            Coverage Diff            @@
##           master     #405     +/-   ##
=========================================
- Coverage   89.06%   88.47%   -0.6%     
=========================================
  Files          49       49             
  Lines        6514     6518      +4     
  Branches     1073     1077      +4     
=========================================
- Hits         5802     5767     -35     
- Misses        470      504     +34     
- Partials      242      247      +5

[larsoner]

At least in the case of:

Requested window size [800 600] but got size (1920, 1200)

The "requested" is implicit from expyfun.json as 800,600 (I set it to this incorrect value just for testing), and the size of the window I got was full-screen 1920x1200 (the actual monitor res)

[drammock]

The "requested" is implicit from expyfun.json as 800,600 (I set it to this incorrect value just for testing), and the size of the window I got was full-screen 1920x1200 (the actual monitor res)

I see. I was misled by the example config line in the PR description, which said "SCREEN_SIZE_PIX": "1920,1200". Your error is from a test of the opposite case (setting config wrong) from what I expected / what motivated this change (setting monitor res. wrong).

[larsoner]

@drammock got motivated to push a doc update to correspond to these changes to try to prevent some future pain (and implicitly document present/past pain), feel free to push if you have changes:

https://692-11614571-gh.circle-artifacts.com/0/html/getting_started.html#configuring-expyfun

@nordme can you paste here or email me the current expyfun.get_config() output? I'd like to have one for a "standard TDT" setup and one for a "standard sound card" setup. I might do this as a separate PR.

[nordme]

Hi Eric,

Here are the contents of that json config file.

{ "AUDIO_CONTROLLER": "tdt", "EXPYFUN_EYELINK": "100.1.1.1", "RESPONSE_DEVICE": "keyboard", "SCREEN_DISTANCE": "100", "SCREEN_WIDTH": "51", "TDT_DELAY": "44", "TDT_INTERFACE": "GB", "TDT_MODEL": "RZ6", "TDT_TRIG_DELAY": "3", "TRIGGER_CONTROLLER": "tdt" }

Lmk if you need anything else.

Erica Peterson Research Scientist Institute for Learning and Brain Sciences University of Washington Box 357988 Seattle, WA 98195 (206) 685-2842

On Fri, Jan 24, 2020 at 3:35 PM Eric Larson notifications@github.com wrote:

@drammock https://github.com/drammock got motivated to push a doc update to correspond to these changes to try to prevent some future pain (and implicitly document present/past pain), feel free to push if you have changes:

https://692-11614571-gh.circle-artifacts.com/0/html/getting_started.html#configuring-expyfun

@nordme https://github.com/nordme can you paste here or email me the current expyfun.get_config() output? I'd like to have one for a "standard TDT" setup and one for a "standard sound card" setup. I might do this as a separate PR.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LABSN/expyfun/pull/405?email_source=notifications&email_token=AJHJNUCZY6XKTBDXHCDAY7DQ7N3MVA5CNFSM4KLJDMYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJ4NCWY#issuecomment-578343259, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJHJNUBD4LVHWT2GCTNBPKLQ7N3MVANCNFSM4KLJDMYA .

[larsoner]

@nordme can you test this out on the machine with the wacky resolution and see if it would have caught the resolution problem?

[larsoner]

Okay here's the updated rendering:

https://693-11614571-gh.circle-artifacts.com/0/html/getting_started.html#configuring-expyfun

@drammock @nordme let me know if this code works whenever you get a chance (not urgent) then we'll merge

[larsoner]

I'll go ahead and merge this, but @nordme @drammock let me know if it doesn't work in testing and I'll make a follow-up PR