-
Notifications
You must be signed in to change notification settings - Fork 180
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
feat(api): Flex Stacker Module Support for EVT #17300
Changes from 13 commits
8a6a0cc
5ffc25a
cfd6b0c
5919b89
ed36a44
1362ce6
f9d30b2
bea6039
1c846f9
0ef16ae
e767735
c986a98
0331330
68a5f74
78dfdcd
f8639ba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1112,21 +1112,77 @@ class FlexStackerContext(ModuleContext): | |
|
||
_core: FlexStackerCore | ||
|
||
@requires_version(2, 23) | ||
def load_labware_to_hopper( | ||
self, | ||
load_name: str, | ||
quantity: int, | ||
label: Optional[str] = None, | ||
namespace: Optional[str] = None, | ||
version: Optional[int] = None, | ||
lid: Optional[str] = None, | ||
) -> None: | ||
"""Load one or more labware onto the flex stacker.""" | ||
self._protocol_core.load_labware_to_flex_stacker_hopper( | ||
module_core=self._core, | ||
load_name=load_name, | ||
quantity=quantity, | ||
label=label, | ||
namespace=namespace, | ||
version=version, | ||
lid=lid, | ||
) | ||
|
||
@requires_version(2, 23) | ||
def enter_static_mode(self) -> None: | ||
"""Enter static mode. | ||
|
||
In static mode, the Flex Stacker will not move labware between the hopper and | ||
the deck, and can be used as a staging slot area. | ||
""" | ||
self._core.set_static_mode(static=True) | ||
|
||
@requires_version(2, 23) | ||
def exit_static_mode(self) -> None: | ||
"""End static mode. | ||
|
||
In static mode, the Flex Stacker will not move labware between the hopper and | ||
the deck, and can be used as a staging slot area. | ||
""" | ||
self._core.set_static_mode(static=False) | ||
|
||
@property | ||
@requires_version(2, 23) | ||
def serial_number(self) -> str: | ||
"""Get the module's unique hardware serial number.""" | ||
return self._core.get_serial_number() | ||
|
||
@requires_version(2, 23) | ||
def retrieve(self) -> None: | ||
def retrieve(self) -> Labware: | ||
"""Release and return a labware at the bottom of the labware stack.""" | ||
self._core.retrieve() | ||
labware_core = self._protocol_core.get_labware_on_module(self._core) | ||
assert labware_core is not None, "Retrieve failed to return labware" | ||
# check core map first | ||
try: | ||
labware = self._core_map.get(labware_core) | ||
except KeyError: | ||
# If the labware is not already in the core map, | ||
# create a new Labware object | ||
labware = Labware( | ||
core=labware_core, | ||
api_version=self._api_version, | ||
protocol_core=self._protocol_core, | ||
core_map=self._core_map, | ||
) | ||
self._core_map.add(labware_core, labware) | ||
return labware | ||
|
||
@requires_version(2, 23) | ||
def store(self, labware: Labware) -> None: | ||
"""Store a labware at the bottom of the labware stack. | ||
|
||
:param labware: The labware object to store. | ||
""" | ||
assert labware._core is not None | ||
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. Is this doing anything? It looks like 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. This is so that user can call this function:
In a refactor, we will add the labware id as a store command param so we can make sure the labware has already been loaded on the stacker slot. 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. Right but I mean, is the |
||
self._core.store() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
from opentrons.hardware_control.modules.types import ( | ||
MagneticBlockModel, | ||
AbsorbanceReaderModel, | ||
FlexStackerModuleModel, | ||
) | ||
from opentrons.legacy_commands import protocol_commands as cmds, types as cmd_types | ||
from opentrons.legacy_commands.helpers import ( | ||
|
@@ -61,6 +62,7 @@ | |
AbstractHeaterShakerCore, | ||
AbstractMagneticBlockCore, | ||
AbstractAbsorbanceReaderCore, | ||
AbstractFlexStackerCore, | ||
) | ||
from .robot_context import RobotContext, HardwareManager | ||
from .core.engine import ENGINE_CORE_API_VERSION | ||
|
@@ -79,6 +81,7 @@ | |
HeaterShakerContext, | ||
MagneticBlockContext, | ||
AbsorbanceReaderContext, | ||
FlexStackerContext, | ||
ModuleContext, | ||
) | ||
from ._parameters import Parameters | ||
|
@@ -94,6 +97,7 @@ | |
HeaterShakerContext, | ||
MagneticBlockContext, | ||
AbsorbanceReaderContext, | ||
FlexStackerContext, | ||
] | ||
|
||
|
||
|
@@ -862,6 +866,9 @@ def load_module( | |
|
||
.. versionchanged:: 2.15 | ||
Added ``MagneticBlockContext`` return value. | ||
|
||
.. versionchanged:: 2.23 | ||
Added ``FlexStackerModuleContext`` return value. | ||
""" | ||
if configuration: | ||
if self._api_version < APIVersion(2, 4): | ||
|
@@ -890,7 +897,18 @@ def load_module( | |
requested_model, AbsorbanceReaderModel | ||
) and self._api_version < APIVersion(2, 21): | ||
raise APIVersionError( | ||
f"Module of type {module_name} is only available in versions 2.21 and above." | ||
api_element=f"Module of type {module_name}", | ||
until_version="2.21", | ||
current_version=f"{self._api_version}", | ||
) | ||
if ( | ||
isinstance(requested_model, FlexStackerModuleModel) | ||
and self._api_version < validation.FLEX_STACKER_VERSION_GATE | ||
): | ||
raise APIVersionError( | ||
api_element=f"Module of type {module_name}", | ||
until_version=str(validation.FLEX_STACKER_VERSION_GATE), | ||
current_version=f"{self._api_version}", | ||
) | ||
|
||
deck_slot = ( | ||
|
@@ -901,7 +919,11 @@ def load_module( | |
) | ||
) | ||
if isinstance(deck_slot, StagingSlotName): | ||
raise ValueError("Cannot load a module onto a staging slot.") | ||
# flex stacker modules can only be loaded into staging slot inside a protocol | ||
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. Is this the right layer to enforce this? We don't want it in the Protocol Engine 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. So we had to do this because the flex stacker is the very first module that provides an addressable area (deck column 4) in a different slot than its cutout mount (column 3). Technically we are still loading the module to column 3 of the deck in the engine (because the engine expects only deck slots). But to users, they wouldn't care which deck cutout the module is being loaded into. It would be weird have user load the module into column 3 in the protocol when the observable addressable area is in column 4. We did this here so that we can keep the same engine behavior for the flex stacker for fast development. In a future refactor, we should use addressable areas as the load location on the context level, instead of physical deck slots. 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. OK, I'm only tenuously following, but I can catch up on this later. It doesn't seem like anything that should block this PR. Thanks for explaining! |
||
if isinstance(requested_model, FlexStackerModuleModel): | ||
deck_slot = validation.convert_flex_stacker_load_slot(deck_slot) | ||
else: | ||
raise ValueError(f"Cannot load {module_name} onto a staging slot.") | ||
|
||
module_core = self._core.load_module( | ||
model=requested_model, | ||
|
@@ -1572,6 +1594,8 @@ def _create_module_context( | |
module_cls = MagneticBlockContext | ||
elif isinstance(module_core, AbstractAbsorbanceReaderCore): | ||
module_cls = AbsorbanceReaderContext | ||
elif isinstance(module_core, AbstractFlexStackerCore): | ||
module_cls = FlexStackerContext | ||
else: | ||
assert False, "Unsupported module type" | ||
|
||
|
Unchanged files with check annotations Beta
units?: string | ||
type?: string | ||
} | ||
interface PipetteQuirksField { | ||
[quirkId: string]: boolean | ||
} | ||
interface QuirksField { | ||
quirks?: PipetteQuirksField | ||
} | ||
export type PipetteSettingsFieldsMap = QuirksField & { | ||
[fieldId: string]: PipetteSettingsField | ||
} | ||
export interface IndividualPipetteSettings { | ||
fields: PipetteSettingsFieldsMap | ||
} | ||
type PipetteSettingsById = Partial<{ [id: string]: IndividualPipetteSettings }> | ||
export type PipetteSettings = PipetteSettingsById | ||
export interface PipetteSettingsUpdateFieldsMap { | ||
[fieldId: string]: PipetteSettingsUpdateField | ||
} | ||
} | null | ||
export interface UpdatePipetteSettingsData { | ||
fields: { [fieldId: string]: PipetteSettingsUpdateField } | ||
} |
export interface ResourceLink { | ||
href: string | ||
meta?: Partial<{ [key: string]: string | null | undefined }> | ||
} | ||
export type ResourceLinks = Record< |
export const appRestart = (message: string): AppRestartAction => ({ | ||
type: APP_RESTART, | ||
payload: { | ||
message: message, | ||
}, | ||
meta: { shell: true }, | ||
}) | ||
export const reloadUi = (message: string): ReloadUiAction => ({ | ||
type: RELOAD_UI, | ||
payload: { | ||
message: message, | ||
}, | ||
meta: { shell: true }, | ||
}) | ||
export const sendLog = (message: string): SendLogAction => ({ | ||
type: SEND_LOG, | ||
payload: { | ||
message: message, | ||
}, | ||
meta: { shell: true }, | ||
}) | ||
export const updateBrightness = (message: string): UpdateBrightnessAction => ({ | ||
type: UPDATE_BRIGHTNESS, | ||
payload: { | ||
message: message, | ||
}, | ||
meta: { shell: true }, | ||
}) |
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.
If I'm reading this correctly, I don't think an
assert
is appropriate here?If you:
retrieve()
Then it will be the case that
labware_core
isNone
, right? So this condition isn't a bug in Opentrons code—it's a problem in the user's protocol that we need to signal with a properEnumeratedError
.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.
The engine retrieve command should have raised an error before we get to this point. I wasn't sure if I should let the engine handle this only or handle it in the module context as well. From your comment, I think the answer is both?
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.
Aah, so any time this
assert
would have raised, you're expecting theself._core.retrieve()
call a couple lines above to have raised, anyway?Okay, that makes sense, and no, I don't think you need to change this
assert
in that case. I would probably leave a comment explaining this, though, or maybe elaborate on it in theassert
message.