Skip to content
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.

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

WIP: iohub updates #3685

Merged
merged 37 commits into from
Mar 19, 2021
Merged

WIP: iohub updates #3685

merged 37 commits into from
Mar 19, 2021

Conversation

isolver
Copy link
Contributor

@isolver isolver commented Mar 17, 2021

RF: Started removing legacy sections of iohub code such as expRuntime class, ability of iohub to run as package outside of psychopy, and the util.clock.py and util.fix_encoding.py files
RF: Some cleaning up of eye tracker config files with an eye toward builder integration.
RF: all iohub demos now use launchHubServer()
ADD: iohub.util.getDeviceNames(): return list of iohub eye tracker names (that launchHubServer takes as keys)
ADD: iohub.util.getDeviceDefaultConfig(): return the eye tracker device config with default values, filtered to show builder visible settings only.
ADD: iohub.util.getDeviceParams(): returns eye tracker config settings similar to getDeviceDefaultConfig, but the value for each setting is a param dict containing valType, inputType, hint, etc...
ENH: eye tracker config file now supports builder_hides setting, which is used to list the device settings that should not be visible in Builder
ENH: iohub device setting can be static, for example manufacturer_name, so that it should be a disabled setting in builder, but still visible.
ENH: gevent tasks scheduling resolution on windows supports sub msec again.

This has been retested with eyelink and gazepoint, as well as keyboard and mouse, so please pull in if possible.

ENH: Make gevent use original libev backend on windows. gevent now defaults to using libuv, which is limited to a 1 msec event loop, causing extra delay in iohub keyboard event timestamping. Switching to use libev on windows makes performance more like older 2.7 distro's.
ENH: improved iohub keyboard/mouse event timing on macOS.

macOS and Windows now both use the 'msgpump_interval' setting in psychopy/iohub/default_config.yaml. Default is 1 msec.

Average iohub keypress timestamp error is down to ~2 msec, use to be 3 - 4+ msec average. Still not as good as psychhid, which is ~1 msec more accurate (i.e. has ~1 msec avg error).
No longer uses iohub ExperimentRuntime class or yaml config files.
And deleted file from source. Now only use launchHubServer() to start iohub
iohub now expects to be a sub package of psychopy and can not be run as a separate package
 removed iohub.util.clock and iohub.util.fix_encoding.
Added some functions that, given a device name, returns the devices matching the name and the config dicts for the device(s). Could be used by Builder to get support eye tracker info from iohub
@codecov
Copy link

codecov bot commented Mar 17, 2021

Codecov Report

Merging #3685 (af2a890) into dev (1bae708) will increase coverage by 0.21%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #3685      +/-   ##
==========================================
+ Coverage   43.27%   43.48%   +0.21%     
==========================================
  Files         274      274              
  Lines       53614    53630      +16     
  Branches     9201     9207       +6     
==========================================
+ Hits        23199    23320     +121     
+ Misses      28090    27953     -137     
- Partials     2325     2357      +32     
Impacted Files Coverage Δ
psychopy/sound/audioclip.py 39.28% <0.00%> (-1.16%) ⬇️
psychopy/sound/microphone.py 39.13% <0.00%> (-0.87%) ⬇️
psychopy/visual/textbox2/fontmanager.py 72.20% <0.00%> (-0.69%) ⬇️
psychopy/tools/wizard.py 72.12% <0.00%> (-0.67%) ⬇️
psychopy/visual/textbox2/textbox2.py 55.63% <0.00%> (+10.14%) ⬆️
psychopy/tools/linebreak.py 56.52% <0.00%> (+27.71%) ⬆️

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 1bae708...af2a890. Read the comment docs.

@coveralls
Copy link

coveralls commented Mar 17, 2021

Coverage Status

Coverage increased (+0.3%) to 47.251% when pulling af2a890 on isolver:wip_iohub_refresh into ae519ab on psychopy:dev.

+ 'manufacturer_name' field should be used as eye tracker name in builder.
Will be used by Builder to know which settings should be hidden from the user, either because they are fixed or because builder may set them internally.
@lgtm-com
Copy link

lgtm-com bot commented Mar 17, 2021

This pull request introduces 3 alerts and fixes 12 when merging 0b67e56 into ae519ab - view on LGTM.com

new alerts:

  • 1 for Module is imported more than once
  • 1 for Except block handles 'BaseException'
  • 1 for Module is imported with 'import' and 'import from'

fixed alerts:

  • 6 for Variable defined multiple times
  • 2 for Unused import
  • 2 for Module is imported with 'import' and 'import from'
  • 1 for Use of the return value of a procedure
  • 1 for Unused local variable

for getDeviceDefaultConfig and getDeviceNames, provding simple example of using each.
iohub eye tracker device manufacturer_name is used by Builder as the name to display in the eye tracker interface selection list.
@lgtm-com
Copy link

lgtm-com bot commented Mar 17, 2021

This pull request introduces 3 alerts and fixes 12 when merging ebbfaf3 into 4d352be - view on LGTM.com

new alerts:

  • 1 for Module is imported more than once
  • 1 for Except block handles 'BaseException'
  • 1 for Module is imported with 'import' and 'import from'

fixed alerts:

  • 6 for Variable defined multiple times
  • 2 for Unused import
  • 2 for Module is imported with 'import' and 'import from'
  • 1 for Use of the return value of a procedure
  • 1 for Unused local variable

needed to be more consistent about use of IOHUB_LIST when needed in supported_config.yaml, so we know if a list should be editable or not.  Also made manufacturer_name a read-only param
RF: Also removed the IOHUB_DATE type from iohub and all yaml files
@lgtm-com
Copy link

lgtm-com bot commented Mar 17, 2021

This pull request introduces 7 alerts and fixes 12 when merging 18dc2be into 4d352be - view on LGTM.com

new alerts:

  • 3 for Unused local variable
  • 1 for Wrong name for an argument in a call
  • 1 for Module is imported more than once
  • 1 for Except block handles 'BaseException'
  • 1 for Module is imported with 'import' and 'import from'

fixed alerts:

  • 6 for Variable defined multiple times
  • 2 for Unused import
  • 2 for Module is imported with 'import' and 'import from'
  • 1 for Use of the return value of a procedure
  • 1 for Unused local variable

First pass at returning a param type dict for each eye tracker setting.
@lgtm-com
Copy link

lgtm-com bot commented Mar 17, 2021

This pull request introduces 4 alerts and fixes 12 when merging c9f47eb into 4d352be - view on LGTM.com

new alerts:

  • 1 for Wrong name for an argument in a call
  • 1 for Module is imported more than once
  • 1 for Except block handles 'BaseException'
  • 1 for Module is imported with 'import' and 'import from'

fixed alerts:

  • 6 for Variable defined multiple times
  • 2 for Unused import
  • 2 for Module is imported with 'import' and 'import from'
  • 1 for Use of the return value of a procedure
  • 1 for Unused local variable

Misc. fixes for changes made so far related to builder integration.
Also removed unneeded iohub base device attributes.
@isolver isolver changed the title WIP: iohub 2021 update WIP: iohub updates Mar 18, 2021
@lgtm-com
Copy link

lgtm-com bot commented Mar 18, 2021

This pull request introduces 4 alerts and fixes 12 when merging 360b0c4 into 4d352be - view on LGTM.com

new alerts:

  • 1 for Module is imported more than once
  • 1 for Except block handles 'BaseException'
  • 1 for Module is imported with 'import' and 'import from'
  • 1 for Wrong number of arguments in a class instantiation

fixed alerts:

  • 6 for Variable defined multiple times
  • 2 for Unused import
  • 2 for Module is imported with 'import' and 'import from'
  • 1 for Use of the return value of a procedure
  • 1 for Unused local variable

RF: removed some python 2.7 specific code
monitor_event_types -> Monitor Event Types
@lgtm-com
Copy link

lgtm-com bot commented Mar 18, 2021

This pull request introduces 1 alert and fixes 12 when merging c7a461e into 4d352be - view on LGTM.com

new alerts:

  • 1 for Unused import

fixed alerts:

  • 6 for Variable defined multiple times
  • 2 for Unused import
  • 2 for Module is imported with 'import' and 'import from'
  • 1 for Use of the return value of a procedure
  • 1 for Unused local variable

- labels are now the full iohub setting path.
- hints are read from a builder_hints.yaml in the device folder, it it exists (only did hints for GP3 so far).
fixes psychopy#3686
@isolver isolver requested a review from mdcutone March 18, 2021 15:37
@isolver
Copy link
Contributor Author

isolver commented Mar 18, 2021

@mdcutone , can you please take a look at this and pull it in if it looks OK to you? I've tested it with the iohub coder demos and nothing is broken, so I think it is good to go into DEV.

was still returning allowedVals: None in some cases.
@lgtm-com
Copy link

lgtm-com bot commented Mar 18, 2021

This pull request introduces 1 alert and fixes 12 when merging af2a890 into 4d352be - view on LGTM.com

new alerts:

  • 1 for Unused import

fixed alerts:

  • 6 for Variable defined multiple times
  • 2 for Unused import
  • 2 for Module is imported with 'import' and 'import from'
  • 1 for Use of the return value of a procedure
  • 1 for Unused local variable

@isolver isolver removed the request for review from mdcutone March 19, 2021 07:21
@isolver isolver merged commit 349c99a into psychopy:dev Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants