python-microscope / microscope

Python library for control of microscope devices, supporting hardware triggers and distribution of devices over the network for performance and flexibility.
https://www.python-microscope.org
GNU General Public License v3.0
67 stars 39 forks source link

Triggers are still a mess. #197

Closed iandobbie closed 3 years ago

iandobbie commented 3 years ago

The simulated camera uses microscope.abc.TRIGGER_SOFT yet the new triggermixin has the enums from microscope.TriggerType and microscope.TriggerMode. Is there any reason to have both incompatible set of definitions for trigger enums?

carandraug commented 3 years ago

The reason is backwards compatibility. All devices that use the TriggerTargetMixin have the set_trigger method and the trigger_mode and trigger_type properties which make use of the TriggerType and TriggerMode enums. In addition, cameras habe the set_trigger_type method which used the same numbers as cockpit.

We could have made a single change that removed one and implemented the other. Not having a transition time would be a problem for programs that made use of this, i.e., it would break Cockpit. In addition, this work was done during COVID when we didn't have access to the actual hardware so I was not keen on removing the old code and left without any fallback.

This is partially related to cockpit#561.

Once we change cockpit to make use of the TriggerTargetMixin (which all cameras now implement), we can remove Camera.get_trigger_type method and the microscope.abc.TRIGGER_* variables.

carandraug commented 3 years ago

I have pushed a change to cockpit so it uses the trigger_type and trigger_mode properties therefore the olf trigger stuff is no longer required. I have pushed e340273 to Microscope now that removes its use. Closing this as fixed.