Skip to content

BF: Add default duration to eyetracker record component#6599

Merged
TEParsons merged 1 commit intopsychopy:devfrom
RebeccaHirst:_addDurationToEyeTrackerRecord
Jun 21, 2024
Merged

BF: Add default duration to eyetracker record component#6599
TEParsons merged 1 commit intopsychopy:devfrom
RebeccaHirst:_addDurationToEyeTrackerRecord

Conversation

@RebeccaHirst
Copy link
Contributor

Eyetracker Record component needs duration in order to stop - so adding a duration value by default so users do not get confused why the experiment halts at the stop record.

Eyetracker Record component needs duration in order to stop - so adding a duration value by default.
@RebeccaHirst RebeccaHirst requested a review from TEParsons June 21, 2024 14:31
@TEParsons TEParsons merged commit 44a581f into psychopy:dev Jun 21, 2024
@TEParsons
Copy link
Contributor

I think this is also fixed by #6567 - which adds a "stop with Routine" parameter. But having a default stop time doesn't conflict so happy for both to happen (just trying to get tests passing on the other one)

@mh105
Copy link
Contributor

mh105 commented Jun 21, 2024

Sorry @RebeccaHirst @TEParsons: This PR is already merged so my comment might be too late. But I don't know if the change here actually has the intended effect. Isn't the default duration of the EyetrackerRecordComponent already set to be 1.0s by the stopType='duration (s)', stopVal=1.0 arguments? I thought the durationEstim parameter is only used for visual display on the timeline as commented here. So does the experiment actually halt when durationEstim is not set? If so I'm worried that there's another bug somewhere that isn't due to durationEstim.

@TEParsons
Copy link
Contributor

Ah, I didn't realise - I thought you'd added a default for duration. Yes, duration estim is (or at least, should) only be used in the UI, the experiment shouldn't be affected

@mh105
Copy link
Contributor

mh105 commented Jun 21, 2024

Might make sense to revert so that users are not confused by why estimated duration defaults to 1s. But more importantly does the experiment actually halt when EyetrackerRecordComponent stops eyetracking recording? That's what I'm worrying about...

@TEParsons
Copy link
Contributor

The only thing I can think is maybe it's leading to the routine being marked as non-slip falsely? Will investigate on Monday

@peircej peircej added the 🐞 bug Issue describes a bug (crash or error) or undefined behavior. label Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐞 bug Issue describes a bug (crash or error) or undefined behavior.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants