-
Notifications
You must be signed in to change notification settings - Fork 39
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 The way to think about This is important, because it means the rest of the system and users of the system only need to care about how To keep this abstraction, though, we need to make sure that we add methods to 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: | ||
|
There was a problem hiding this comment.
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 andif
for stage creation below, I think what we should do is:import squid.stage
heresquid.stage.__init__.py
, add aget_stage(stage_config: StageConfig)
.stage_type: StageType
field toStageConfig
so we can define what stage type we're using. Add aclass StageType(enum.Enum)
if we don't have it!The reasons I'd suggest this are:
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 anAbstractStage
concept. Users only need to be aware of theAbstractStage
and the interface it represents, and then our software can take care of giving them the correct instance under the hood.squid.stage.get_stage(...)
, then it won't need to be repeated every time someone wants to create a stage.