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

fix bug for camera not pausing in scopeless mode #122

Merged
merged 7 commits into from
Feb 19, 2021
Merged

Conversation

kkoetter
Copy link
Contributor

Changed the toggling between camera_loop and pause_loop in CameraProcess to avoid camera getting stuck in camera loop for the mock camera. Issue with --scopeless was that the Camera was just entering pause_loop at initialisation of the GUI and afterwards was in camera_loop without a return. Code needs to be tested with hamamatsu camera to verify changes didn't crash anything. There are multiple CameraMode that are created but never checked such as TRIGGERED, EXPERIMENT_RUNNING. Could this potentially be simplified?

@kkoetter kkoetter changed the title fix bug for camera not pausing in scales mode fix bug for camera not pausing in scopeless mode Feb 18, 2021
@kkoetter
Copy link
Contributor Author

Update: Tested with hamamatsu in the light sheet computer. No apparent problems so far.

@@ -21,8 +21,8 @@

class CameraMode(Enum):
PREVIEW = 1
TRIGGERED = 2
EXPERIMENT_RUNNING = 3
#TRIGGERED = 2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If @vilim does not object I would entirely remove those

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be no modes that are not used, and no commented lines

#if self.parameters.camera_mode == CameraMode.PAUSED:
self.pause_loop()
#else:
self.camera.start_acquisition()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the logger should also get a "Stopped acquisition" message after the camera_loop is left, and self.camera.stop_acquisition() should be called. Actually, the camera start/stop and the logging should probably go in the camera_loop() and the pause_loop() methods, after breaking from the respective while loops!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also please remove the commented lines and write a small comment why the pause_loop is enough (basically inside of it the if you removed above is implemented.

Copy link
Member

@vilim vilim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, after these few changes it can be merged!

@@ -21,8 +21,8 @@

class CameraMode(Enum):
PREVIEW = 1
TRIGGERED = 2
EXPERIMENT_RUNNING = 3
#TRIGGERED = 2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be no modes that are not used, and no commented lines

#if self.parameters.camera_mode == CameraMode.PAUSED:
self.pause_loop()
#else:
self.camera.start_acquisition()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also please remove the commented lines and write a small comment why the pause_loop is enough (basically inside of it the if you removed above is implemented.

@kkoetter
Copy link
Contributor Author

kkoetter commented Feb 19, 2021

I now have removed the unnecessary CameraModes and restructured the logger into the camera_loop and pause_loop to clean up the run_camera function @vigji . If thee is no more changes requested I would be happy for someone to review it again :)

@vigji vigji merged commit f5e199a into master Feb 19, 2021
@vigji vigji deleted the camera-paused-bug-fix branch February 19, 2021 10:23
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