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

BF: fix an issue with etRecord component for Stop Only #6460

Closed
wants to merge 1 commit into from

Conversation

mh105
Copy link
Contributor

@mh105 mh105 commented May 14, 2024

There is a bug with the etRecord component in builder because the constructor for psychopy.hardware.eyetracker.EyetrackerControl always initializes the _status attribute as NOT_STARTED. This causes an issue when the component is used under the "Stop Only" Record actions. In this scenario, two independent etRecord components are initialized, causing the bug, because the experiment can only change the status of the second component (supposed to stop recording) to FINISHED if the status is already STARTED.

Instead, the status of any EyetrackerControl instance should reflect whether recording is currently enabled for the tracker instance passed into the constructor. Looking through psychopy.iohub.devices.eyetracker.hw, isRecordingEnabled() is appropriately defined for everyone so this fix should work seamlessly.

@peircej
Copy link
Member

peircej commented May 14, 2024

@RebeccaHirst @TEParsons this is an even shorter changed compared to your solutions in #6457 and #6456

Is this going to be sufficient for the short-term BF (postponing the other discussion about whether "start/stop" times refer to the start of the component versus the start of the eyetracker?

@TEParsons TEParsons changed the base branch from dev to release May 15, 2024 10:25
@TEParsons TEParsons changed the base branch from release to dev May 15, 2024 10:26
@TEParsons
Copy link
Contributor

Yep sounds like a plan :) @mh105 would you mind creating a new branch from psychopy/release and cherry picking this change across? Then we can pull this in for the next bug fix release and continue working on the other stuff @RebeccaHirst and I are figuring out in the dev branch

@mh105
Copy link
Contributor Author

mh105 commented May 15, 2024

Sure. Just created a new PR to psychopy/release. I'll close this now.

@mh105 mh105 closed this May 15, 2024
@mh105 mh105 deleted the etRecord_stop_only branch May 16, 2024 03:08
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

3 participants