FF: skip ioHub event callback without server or PsychoPy window

[mh105]

There is a very niche bug within ioHub at the end of an experiment on Windows. When the ioHub server itself has closed or the PsychoPy window has closed but the rest of close-down steps are still going, if users move the mouse at this moment, it triggers a Traceback error due to trying to access attributes from None type.

p.s. It's still a bug on Linux but the Linux implementation is wrapped by a try statement so does not lead to a crush of the processes.

This FF allows the _nativeEventCallback method to return True and ignore the event callback if no ioHub server or PsychoPy window is currently open. This fix allows both the ioHub server subprocess and PsychoPy main process to close out gracefully.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 50.56%. Comparing base (8c041c4) to head (754edc0).
Report is 17 commits behind head on release.

Additional details and impacted files

@@           Coverage Diff            @@
##           release    #6949   +/-   ##
========================================
  Coverage    50.55%   50.56%           
========================================
  Files          332      332           
  Lines        61217    61220    +3     
========================================
+ Hits         30951    30954    +3     
  Misses       30266    30266           

Components	Coverage Δ
app	∅ <ø> (∅)
boilerplate	∅ <ø> (∅)
library	∅ <ø> (∅)
vm-safe library	∅ <ø> (∅)

[peircej]

Wow, we have heard of issues where iohub seemed not to close down properly, but I wonder if that's the reason! 👀 😮

[TEParsons]

If this is the reason my mouse sometimes moves at a snails pace when VS Code Debugger catches an error (and I have to very slowly drag it to the "Continue" button) I'll be very grateful :')

I wonder, would it make sense to have this check inside isReportingEvents? If there's no ioHub server, then wouldn't we want isReportingEvents to return False?

[mh105]

@TEParsons I can't say I ever fully understood these ioHub-related codes in PsychoPy, but from my reading of the code, this _nativeEventCallback method of the Mouse class must either return True or return some kind of event. If you look at the linux2 and win32 implementations of this method, it never returns False even when if self.isReportingEvents() is False. My guess is that this _nativeEventCallback is called along the way of some other handling of callback events so it either updates some attributes of the Mouse instance when the event needs to get reported, or just returns True to move things along.

I just read through the implementations again and found this comment line that corroborates my hypothesis:

# pyHook require the callback to return True to inform the windows
# low level hook functionality to pass the event on.

So yeah... I think it's probably best to just return True here like the rest of the _nativeEventCallback is doing. In my testing this commit fixes the Traceback error and works as intended.

EDITED: if you are wondering whether my added if statement should be outside the if self.isReportingEvents loop, I just didn't want to mess with the self.isReportingEvents attribute. The way I'm returning True early right now is identical to the behavior of _nativeEventCallback when self.isReportingEvents == False. So yeah my guess is that moving it out of the if loop would work the same but may make our future selves a bit confused about the priority of self.isReportingEvents.

[mh105]

Have you had a chance to see if this PR fixes the issue? Let me know if there's any additional change you'd like me to make before merging. Thanks!

[peircej]

@mh105 we aren't actually certain this was causing issues for any of us, but the check looks wise and I've merged it anyway
thanks :-)