agencyenterprise / neural-data-simulator

Electrophysiology data simulator for developing brain-computer interfaces
https://agencyenterprise.github.io/neural-data-simulator/
Apache License 2.0
71 stars 7 forks source link

Fix Subprocesses Cleanup on run_closed_loop #22

Closed eudesf closed 1 year ago

eudesf commented 1 year ago

Introduction

Abrupt termination of run_closed_loop with Ctrl+C can leave LSL streams undeleted and processes running in the background. Also, when the center_out_reach script terminates the main experiment task, the streams are still running while the metrics window "Velocities overview" is showing.

Changes

Behavior

On run_closed_loop:

codecov-commenter commented 1 year ago

Codecov Report

Patch and project coverage have no change.

Comparison is base (4fb8ffa) 86.99% compared to head (0c58d07) 86.99%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #22 +/- ## ======================================= Coverage 86.99% 86.99% ======================================= Files 21 21 Lines 1707 1707 ======================================= Hits 1485 1485 Misses 222 222 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

eudesf commented 1 year ago

@NewtonSander I just found that the os.mkfifo() doesn't work on Windows. I will switch to os.pipe() then.

NewtonSander commented 1 year ago

@eudesf I made some changes in a forked branch, could you check it? https://github.com/agencyenterprise/neural-data-simulator/blob/run_closed_loop_subprocess_cleanup2/src/tasks/run_closed_loop.py

eudesf commented 1 year ago

@eudesf I made some changes in a forked branch, could you check it? https://github.com/agencyenterprise/neural-data-simulator/blob/run_closed_loop_subprocess_cleanup2/src/tasks/run_closed_loop.py

Thanks for creating an example! But I think we got into the same problem of function scoping or naming. Have you noticed that the _wait_for_center_out_reach_to_finish can actually finish leaving the center_out_reach process still running without raising any exception as if it finished successfully? And we are checking on check on line 115 to see if it's still running, which is kinda confusing as you already waited for it.