BF: Unregister event monitors iohub calibration

[mh105]

This PR is going to undo commit 9808929, which I believe is an incorrect commit.

If we don't remove the registered EventMonitors instantiated by the calibration instance, they are going to keep hijacking key releases to space and escape because of this event handler:

psychopy/psychopy/iohub/devices/eyetracker/calibration/procedure.py

Line 122 in 7969ebb

def _handleEvent(self, event):

It might have appeared like calibration._unregisterEventMonitors() was not required anymore, but that's likely because the keyboard events was set to press instead of release.

One can test out in 2024.1.4: after the eyetracker calibration component, key releases to space and escape no longer work as intended.

@peircej I noticed that this same line has been commented out in the separate eye tracking plugin packages such as in psychopy-eyetracker-sr-research:

# genv._unregisterEventMonitors()

It would be great if those pip packages could be updated for this issue as well.

[peircej]

Hmm, this is tricky. We (with @mdcutone ) had found that with the unregister call in place that was blocking access to the keyboard, which is why it was removed. But now you found that removing it is blocking access? 🤔
I wonder if this depends on what keyboard backend is being used for the main experiment.

[mh105]

Hi Jon, from the implementation of BaseCalibrationProcedure._unregisterEventMonitors(), I couldn't quite see how it would block access since it only has access to a keyboard device through self._eyetracker._iohub_server.devices. If we look at how event listeners are removed, they are only removed from the calibration instance and should be independent of other event listeners tied to the main PsychoPy window. These event listeners are also freshly created by the BaseCalibrationProcedure constructor through the ._registerEventMonitors() method.

Could you see if you can reproduce the following bug (which prompted this BF PR) that I'm seeing on my end with PsychoPy standalone version 2024.1.4:

1. Create a blank builder experiment
2. Select MouseGaze for Eyetracking under experiment setting
3. Add an Eyetracker Calibration component
4. Add another routine after this calibration component
5. Add a simple text stimuli that only has start time
6. Add a keyboard component that 1) with 'Force end Routine' checked, 2) Register keypress on... release, and 3) has Allowed keys to be ['space', 'p']

What you should see is that after the calibration procedure completes and when the experiment has progressed to the text display routine, pressing and releasing space does not end the routine, while pressing and releasing p does. I believe this is because with _unregisterEventMonitors() commented out, the event handler in the background keeps processing key release events to space and escape. Since it always calls .clearAllEventBuffers() afterwards, it effectively hijacks certain keyboard releases.

When I resurrect _unregisterEventMonitors(), everything seems to be working properly, and I have been using ioHub as the keyboard backend. If you can reproduce this bug but uncommenting _unregisterEventMonitors() was blocking access to keyboard (although I couldn't see the mechanism of that), you can try adding

        del calibration.window
        del calibration

as I included in the commit and see if they help.

[peircej]

Yes, thanks @mh105 we are actively investigating. What you say sounds correct - we just want to check that this doesn't introduce a new problem 😁

[mh105]

I was working on getting ioHub eyetracker working, which forced me to understand how the ioHub devices are managed. This also led me to look at the psychopy.hardware classes more closely. It seems that some of the methods such as .start() and .stop() has no effect when the ioHub backend is used for a KeyboardDevice instance.

I want to drop a note here in case this is related to the PR here. I also want to ask if this IS the intended design of this part of PsychoPy? It seems a bit dangerous if users create Keyboards instances and calling the start and stop methods would confusingly have no effect. But I now do understand that the _iohubKeyboard is a bit clunky to work with... so not sure what's your recommendation here.

[peircej]

I was working on getting ioHub eyetracker working, which forced me to understand how the ioHub devices are managed. This also led me to look at the psychopy.hardware classes more closely. It seems that some of the methods such as .start() and .stop() has no effect when the ioHub backend is used for a KeyboardDevice instance.

I want to drop a note here in case this is related to the PR here. I also want to ask if this IS the intended design of this part of PsychoPy? It seems a bit dangerous if users create Keyboards instances and calling the start and stop methods would confusingly have no effect. But I now do understand that the _iohubKeyboard is a bit clunky to work with... so not sure what's your recommendation here.

I don't think it's so much that iohub is the odd one here. The issue is that psychtoolbox exposed the option to start/stop reading from the keyboard, which we had never thought of as a necessary feature and in some backends wouldn't really be possible at all. So pyglet and pygame also don't have a notion of stopping the keyboard events either. Builder scripts do start/stop listening to keyboards but under the hood, even when teh builder Keyboard Comp has stopped listening the events are still coming in

[peircej]

Regarding the original issue, we are still completely puzzled. We can't generate the failure in either direction. On our machines the line can be present or absent and have no effect 🤷 so it seems like some combination of factors that give rise to this having an impact. We'll keep our eyes open to new users complaining that things freeze if they have calibration/validation routines and take it from there

[mh105]

Thanks a lot @peircej for looking into this! Interestingly not calling _unregisterEventMonitors() with the SR research eyetracker doesn't have any effect... 🥲 But with mouseGaze, not having this line hijacks all escape and space key releases after calibration/validation routines for me on macOS with ioHub backend.

I have a good hunch that this strange behavior comes from how the last existing Keyboard device in sequence is connected instead of a new Keyboard gets added here. So a new event listener is added to the existing kbDevice.

In contrast, with SR research eyetracker, a new keyboard device gets added to _iohub_server since it needs to listen to keyboard from the Eyelink Host PC. So adding new event listeners to this separate kbDevice doesn't impact existing keyboard devices. Nevertheless, it's probably a good idea to unregister it as well at the end of calibration.

P.S.: As much as I admire the use of servers and threads in ioHub, it has made my head spin whenever I need to read through iohub related codes...