-
Notifications
You must be signed in to change notification settings - Fork 199
feat(api): store and use disposal locations in the location cache #18268
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
Changes from all commits
0a40685
65f4798
4c92c12
baa81d9
a6ed3f7
c832eda
3bf9bc8
07d0199
f0b74b8
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 |
|---|---|---|
|
|
@@ -258,7 +258,7 @@ def aspirate( | |
| "knows where it is." | ||
| ) from e | ||
|
|
||
| if isinstance(target, (TrashBin, WasteChute)): | ||
| if isinstance(target, validation.DisposalTarget): | ||
| raise ValueError( | ||
| "Trash Bin and Waste Chute are not acceptable location parameters for Aspirate commands." | ||
| ) | ||
|
|
@@ -399,6 +399,10 @@ def dispense( | |
|
|
||
| .. versionchanged:: 2.17 | ||
| Behavior of the ``volume`` parameter. | ||
|
|
||
| .. versionchanged:: 2.24 | ||
| ``location`` is no longer required if the pipette just moved to, dispensed, or blew out | ||
| into a trash bin or waste chute. | ||
| """ | ||
| if self.api_version < APIVersion(2, 15) and push_out: | ||
| raise APIVersionError( | ||
|
|
@@ -432,24 +436,24 @@ def dispense( | |
|
|
||
| flow_rate = self._core.get_dispense_flow_rate(rate) | ||
|
|
||
| if isinstance(target, (TrashBin, WasteChute)): | ||
| if isinstance(target, validation.DisposalTarget): | ||
| with publisher.publish_context( | ||
| broker=self.broker, | ||
| command=cmds.dispense_in_disposal_location( | ||
| instrument=self, | ||
| volume=c_vol, | ||
| location=target, | ||
| location=target.location, | ||
| rate=rate, | ||
| flow_rate=flow_rate, | ||
| ), | ||
| ): | ||
| self._core.dispense( | ||
| volume=c_vol, | ||
| rate=rate, | ||
| location=target, | ||
| location=target.location, | ||
| well_core=None, | ||
| flow_rate=flow_rate, | ||
| in_place=False, | ||
| in_place=target.in_place, | ||
| push_out=push_out, | ||
| meniscus_tracking=None, | ||
| ) | ||
|
|
@@ -656,6 +660,10 @@ def blow_out( | |
| without first calling a method that takes a location, like | ||
| :py:meth:`.aspirate` or :py:meth:`dispense`. | ||
| :returns: This instance. | ||
|
|
||
| .. versionchanged:: 2.24 | ||
| ``location`` is no longer required if the pipette just moved to, dispensed, or blew out | ||
| into a trash bin or waste chute. | ||
| """ | ||
| well: Optional[labware.Well] = None | ||
| move_to_location: types.Location | ||
|
|
@@ -696,17 +704,17 @@ def blow_out( | |
| well = target.well | ||
| elif isinstance(target, validation.PointTarget): | ||
| move_to_location = target.location | ||
| elif isinstance(target, (TrashBin, WasteChute)): | ||
| elif isinstance(target, validation.DisposalTarget): | ||
| with publisher.publish_context( | ||
| broker=self.broker, | ||
| command=cmds.blow_out_in_disposal_location( | ||
| instrument=self, location=target | ||
| instrument=self, location=target.location | ||
| ), | ||
| ): | ||
| self._core.blow_out( | ||
| location=target, | ||
| location=target.location, | ||
| well_core=None, | ||
| in_place=False, | ||
| in_place=target.in_place, | ||
| ) | ||
| return self | ||
|
|
||
|
|
@@ -793,8 +801,12 @@ def touch_tip( # noqa: C901 | |
| # If location is a valid well, move to the well first | ||
| if location is None: | ||
| last_location = self._protocol_core.get_last_location() | ||
| if not last_location: | ||
| raise RuntimeError("No valid current location cache present") | ||
| if last_location is None or isinstance( | ||
| last_location, (TrashBin, WasteChute) | ||
| ): | ||
| raise RuntimeError( | ||
| f"Cached location of {last_location} is not valid for touch tip." | ||
| ) | ||
| parent_labware, well = last_location.labware.get_parent_labware_and_well() | ||
| if not well or not parent_labware: | ||
| raise RuntimeError( | ||
|
|
@@ -896,10 +908,10 @@ def air_gap( # noqa: C901 | |
|
|
||
| .. versionchanged:: 2.22 | ||
| No longer implemented as an aspirate. | ||
|
|
||
| .. versionchanged:: 2.24 | ||
| Adds the ``rate`` and ``flow_rate`` parameter. You can only define one or the other. If | ||
| both are unspecified then ``rate`` is by default set to 1.0. | ||
| Can air gap over a trash bin or waste chute. | ||
| """ | ||
| if not self._core.has_tip(): | ||
| raise UnexpectedTipRemovalError("air_gap", self.name, self.mount) | ||
|
|
@@ -923,10 +935,23 @@ def air_gap( # noqa: C901 | |
|
|
||
| if height is None: | ||
| height = 5 | ||
| loc = self._protocol_core.get_last_location() | ||
| if not loc or not loc.labware.is_well: | ||
| raise RuntimeError("No previous Well cached to perform air gap") | ||
| target = loc.labware.as_well().top(height) | ||
| last_location = self._protocol_core.get_last_location() | ||
| if self.api_version < APIVersion(2, 24) and isinstance( | ||
| last_location, (TrashBin, WasteChute) | ||
| ): | ||
| last_location = None | ||
|
Contributor
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. Did you mean to use
Contributor
Author
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. See above comment.
Contributor
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. (Sorry, I don't know why Github sent out a duplicate comment here!) |
||
| if last_location is None or ( | ||
| isinstance(last_location, types.Location) | ||
| and not last_location.labware.is_well | ||
| ): | ||
| raise RuntimeError( | ||
| f"Cached location of {last_location} is not valid for air gap." | ||
| ) | ||
| target: Union[types.Location, TrashBin, WasteChute] | ||
| if isinstance(last_location, types.Location): | ||
| target = last_location.labware.as_well().top(height) | ||
| else: | ||
| target = last_location.top(height) | ||
| self.move_to(target, publish=False) | ||
| if self.api_version >= _AIR_GAP_TRACKING_ADDED_IN: | ||
| self._core.prepare_to_aspirate() | ||
|
|
@@ -2583,14 +2608,22 @@ def well_bottom_clearance(self) -> "Clearances": | |
| """ | ||
| return self._well_bottom_clearances | ||
|
|
||
| def _get_last_location_by_api_version(self) -> Optional[types.Location]: | ||
| def _get_last_location_by_api_version( | ||
| self, | ||
| ) -> Optional[Union[types.Location, TrashBin, WasteChute]]: | ||
| """Get the last location accessed by this pipette, if any. | ||
|
|
||
| In pre-engine Protocol API versions, this call omits the pipette mount. | ||
| Between 2.14 (first engine PAPI version) and 2.23 this only returns None or a Location object. | ||
| This is to preserve pre-existing, potentially buggy behavior. | ||
| """ | ||
| if self._api_version >= ENGINE_CORE_API_VERSION: | ||
| if self._api_version >= APIVersion(2, 24): | ||
| return self._protocol_core.get_last_location(mount=self._core.get_mount()) | ||
| elif self._api_version >= ENGINE_CORE_API_VERSION: | ||
| last_location = self._protocol_core.get_last_location( | ||
| mount=self._core.get_mount() | ||
| ) | ||
| return last_location if isinstance(last_location, types.Location) else None | ||
| else: | ||
| return self._protocol_core.get_last_location() | ||
|
Contributor
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. What does this return (before version 2.14)? Could this be a trashbin/wastechute?
Contributor
Author
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 calls |
||
|
|
||
|
|
@@ -2646,7 +2679,11 @@ def configure_for_volume(self, volume: float) -> None: | |
| actual_value=str(volume), | ||
| ) | ||
| last_location = self._get_last_location_by_api_version() | ||
| if last_location and isinstance(last_location.labware, labware.Well): | ||
| if ( | ||
| last_location | ||
| and isinstance(last_location, types.Location) | ||
| and isinstance(last_location.labware, labware.Well) | ||
| ): | ||
| self.move_to(last_location.labware.top()) | ||
|
Contributor
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 the pipette should not move during
Contributor
Author
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. The main concern for |
||
| self._core.configure_for_volume(volume) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1190,9 +1190,16 @@ def home(self) -> None: | |
| self._core.home() | ||
|
|
||
| @property | ||
| def location_cache(self) -> Optional[Location]: | ||
| """The cache used by the robot to determine where it last was.""" | ||
| return self._core.get_last_location() | ||
| def location_cache(self) -> Optional[Union[Location, TrashBin, WasteChute]]: | ||
| """The cache used by the robot to determine where it last was. | ||
|
|
||
| .. versionchanged:: 2.24 | ||
|
Contributor
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. We've hidden this reference entry, but this is a good addition for internal documentation. |
||
| Can return a ``TrashBin`` or ``WasteChute`` object. | ||
| """ | ||
| last_loc = self._core.get_last_location() | ||
| if isinstance(last_loc, Location) or self._api_version >= APIVersion(2, 24): | ||
| return last_loc | ||
| return None | ||
|
|
||
| @location_cache.setter | ||
| def location_cache(self, loc: Optional[Location]) -> None: | ||
|
|
||
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.
Did you mean to use
_get_last_location_by_api_version()that you defined below?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 we were to make this change we'd call
get_last_locationusing a mount argument, which we have not been doing here. That may be more technically correct, but this is what has been in the code and changing it without a version check would potentially break or change the behavior of older protocols, and changing that is slightly out of scope for this PR.