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

Prior stage with abstract stage class #65

Merged
merged 3 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions software/control/gui_hcs.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
import squid.abc
import squid.logging
import squid.config
import squid.stage.prior
import squid.stage.cephla
import squid.stage.utils
import control.microscope
from control.microscope import LightSourceType, IntensityControlMode, ShutterControlMode
Expand All @@ -29,6 +27,10 @@

import control.filterwheel as filterwheel

if USE_PRIOR_STAGE:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of doing this if with hidden imports here and if for stage creation below, I think what we should do is:

  • import squid.stage here
  • in squid.stage.__init__.py, add a get_stage(stage_config: StageConfig).
  • If we don't have it already, add a stage_type: StageType field to StageConfig so we can define what stage type we're using. Add a class StageType(enum.Enum) if we don't have it!

The reasons I'd suggest this are:

  • We don't want users of the AbstractStage concept to need to care about how/when to build the different stage variants - we want to hide that from them! That's the reason for having an AbstractStage concept. Users only need to be aware of the AbstractStage and the interface it represents, and then our software can take care of giving them the correct instance under the hood.
  • Since it's going to be common for users to create a stage (eg: if they're writing a headless script), we don't want this logic to need to be repeated in each of those cases. If we instead implement it once in a squid.stage.get_stage(...), then it won't need to be repeated every time someone wants to create a stage.

import squid.stage.prior
else:
import squid.stage.cephla

if CAMERA_TYPE == "Toupcam":
try:
Expand Down Expand Up @@ -78,9 +80,6 @@
else:
import control.camera as camera_fc

if USE_PRIOR_STAGE:
from control.stage_prior import PriorStage

import control.core.core as core
import control.microcontroller as microcontroller
import control.serial_peripherals as serial_peripherals
Expand Down Expand Up @@ -199,9 +198,17 @@ def loadObjects(self, is_simulation):
self.liveController = core.LiveController(
self.camera, self.microcontroller, self.configurationManager, self.illuminationController, parent=self
)
self.stage: squid.abc.AbstractStage = squid.stage.cephla.CephlaStage(
microcontroller=self.microcontroller, stage_config=squid.config.get_stage_config()
)

if USE_PRIOR_STAGE:
self.stage: squid.abc.AbstractStage = squid.stage.prior.PriorStage(
sn=PRIOR_STAGE_SN
)

else:
self.stage: squid.abc.AbstractStage = squid.stage.cephla.CephlaStage(
microcontroller=self.microcontroller, stage_config=squid.config.get_stage_config()
)

self.slidePositionController = core.SlidePositionController(
self.stage, self.liveController, is_for_wellplate=True
)
Expand Down Expand Up @@ -409,13 +416,6 @@ def loadHardwareObjects(self):
self.log.error("Error initializing Optospin Emission Filter Wheel")
raise

if USE_PRIOR_STAGE:
try:
self.priorstage = PriorStage(PRIOR_STAGE_SN, parent=self)
except Exception:
self.log.error("Error initializing Prior Stage")
raise

def setupHardware(self):
# Setup hardware components
if USE_ZABER_EMISSION_FILTER_WHEEL:
Expand Down
272 changes: 0 additions & 272 deletions software/control/stage_prior.py

This file was deleted.

6 changes: 3 additions & 3 deletions software/control/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -1541,17 +1541,17 @@ def move_z_backward(self):
self.stage.move_z(-self.entry_dZ.value() / 1000)

def set_deltaX(self, value):
mm_per_ustep = 1.0 / self.stage.get_config().X_AXIS.convert_real_units_to_ustep(1.0)
mm_per_ustep = 1.0 / self.stage.x_mm_to_usteps(1.0)
deltaX = round(value / mm_per_ustep) * mm_per_ustep
self.entry_dX.setValue(deltaX)

def set_deltaY(self, value):
mm_per_ustep = 1.0 / self.stage.get_config().Y_AXIS.convert_real_units_to_ustep(1.0)
mm_per_ustep = 1.0 / self.stage.y_mm_to_usteps(1.0)
deltaY = round(value / mm_per_ustep) * mm_per_ustep
self.entry_dY.setValue(deltaY)

def set_deltaZ(self, value):
mm_per_ustep = 1.0 / self.stage.get_config().Z_AXIS.convert_real_units_to_ustep(1.0)
mm_per_ustep = 1.0 / self.stage.z_mm_to_usteps(1.0)
deltaZ = round(value / 1000 / mm_per_ustep) * mm_per_ustep * 1000
self.entry_dZ.setValue(deltaZ)

Expand Down
9 changes: 9 additions & 0 deletions software/squid/stage/cephla.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@ def _configure_axis(self, microcontroller_axis_number: int, axis_config: AxisCon
)
self._microcontroller.turn_on_stage_pid(microcontroller_axis_number)

def x_mm_to_usteps(self, mm: float):
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we want to add these (we may not want to since usteps is a concept that only makes sense when using stepper motors. If we ever have stages that don't use steppers it'll be confusing), we should add all 3 to the AbstractStage interface as abstract methods.

The way to think about AbstractStage is that it is the only structure the rest of our system can know about. It defines the interface between all other parts of the system, and stages.

This is important, because it means the rest of the system and users of the system only need to care about how AbstractStage is defined, and don't need to know any the nitty gritty details of how the individual Stage implementations work.

To keep this abstraction, though, we need to make sure that we add methods to AbstractStage if they're going to be used elsewhere in the system.

Also it's generally a good idea to "keep your interfaces as small as possible" -- don't add methods/attributes/etc if you don't need to -- because these interfaces are what we need to support and maintain. The smaller the surface area we need to maintain, the better.

return self._config.X_AXIS.convert_real_units_to_ustep(mm)

def y_mm_to_usteps(self, mm: float):
return self._config.Y_AXIS.convert_real_units_to_ustep(mm)

def z_mm_to_usteps(self, mm: float):
return self._config.Z_AXIS.convert_real_units_to_ustep(mm)

def move_x(self, rel_mm: float, blocking: bool = True):
self._microcontroller.move_x_usteps(self._config.X_AXIS.convert_real_units_to_ustep(rel_mm))
if blocking:
Expand Down
Loading
Loading