Skip to content

Commit

Permalink
refactor(api): Express labware offset locations as sequences (#17363)
Browse files Browse the repository at this point in the history
This is a pattern that we're going to adopt for symbolic geometry
locations in HTTP API contexts - expressing them as a sequence of
heterogenous unions that represent kinds of places things can be. We'll
do this for locations of labware instances in a followup, but for now
we're doing it for offset locations.

Offset locations were previously objects that had a slot name; an
optional module model, for a location on a module; and an optional
labware URI, for a location on a labware (possibly an adapter, possibly
on a model). This can't represent labware stacks, and it's locked to
slot names. These are more or less fine for now still, but it would be
nice to support stacks in the general case, and it will be awkward for
clients to use the object-with-optional-attributes model alongside the
sequence-of-locations instance model. Plus, while this won't need a
database schema change for the offsets stored in runs, it _will_ for the
offsets api - so let's make this change now while that schema and API
haven't yet shipped.

## Testing
- [x] do the offsets still apply, on
   - [x] a slot
   - [x] a module
   - [x] an adapter on a model

Since the HTTP API still is the same and everything is tested
internally, it is sufficient to do normal LPC testing - everything
should still work.

## Reviews
- [ ] look appropriate? Missing anything?
  • Loading branch information
sfoster1 authored Jan 29, 2025
1 parent 16bd893 commit 5303a3b
Show file tree
Hide file tree
Showing 76 changed files with 2,080 additions and 474 deletions.
4 changes: 2 additions & 2 deletions api-client/src/maintenance_runs/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type {
} from '@opentrons/shared-data'
import type {
RunCommandSummary,
LabwareOffsetCreateData,
LegacyLabwareOffsetCreateData,
RunStatus,
RunAction,
} from '../runs'
Expand Down Expand Up @@ -42,7 +42,7 @@ export interface MaintenanceRunError {
}

export interface CreateMaintenanceRunData {
labwareOffsets?: LabwareOffsetCreateData[]
labwareOffsets?: LegacyLabwareOffsetCreateData[]
}

export interface LabwareDefinitionSummary {
Expand Down
6 changes: 3 additions & 3 deletions api-client/src/runs/createLabwareOffset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ import { POST, request } from '../request'

import type { ResponsePromise } from '../request'
import type { HostConfig } from '../types'
import type { LabwareOffsetCreateData, Run } from './types'
import type { LegacyLabwareOffsetCreateData, Run } from './types'

export function createLabwareOffset(
config: HostConfig,
runId: string,
data: LabwareOffsetCreateData
data: LegacyLabwareOffsetCreateData
): ResponsePromise<Run> {
return request<Run, { data: LabwareOffsetCreateData }>(
return request<Run, { data: LegacyLabwareOffsetCreateData }>(
POST,
`/runs/${runId}/labware_offsets`,
{ data },
Expand Down
4 changes: 2 additions & 2 deletions api-client/src/runs/createRun.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ import type { ResponsePromise } from '../request'
import type { HostConfig } from '../types'
import type {
Run,
LabwareOffsetCreateData,
LegacyLabwareOffsetCreateData,
RunTimeParameterValuesCreateData,
RunTimeParameterFilesCreateData,
} from './types'

export interface CreateRunData {
protocolId?: string
labwareOffsets?: LabwareOffsetCreateData[]
labwareOffsets?: LegacyLabwareOffsetCreateData[]
runTimeParameterValues?: RunTimeParameterValuesCreateData
runTimeParameterFiles?: RunTimeParameterFilesCreateData
}
Expand Down
30 changes: 26 additions & 4 deletions api-client/src/runs/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ export interface LabwareOffset {
id: string
createdAt: string
definitionUri: string
location: LabwareOffsetLocation
location: LegacyLabwareOffsetLocation
locationSequence?: LabwareOffsetLocationSequence
vector: VectorOffset
}

Expand Down Expand Up @@ -156,14 +157,35 @@ export interface CreateRunActionData {
actionType: RunActionType
}

export interface LabwareOffsetLocation {
export interface OnAddressableAreaLabwareOffsetLocationSequenceComponent {
kind: 'onAddressableArea'
labware: string
}

export interface OnModuleOffsetLocationSequenceComponent {
kind: 'onModule'
moduleModel: ModuleModel
}

export interface OnLabwareOffsetLocationSequenceComponent {
kind: 'onLabware'
labwareUri: string
}

export type LabwareOffsetLocationSequenceComponent =
| OnAddressableAreaLabwareOffsetLocationSequenceComponent
| OnModuleOffsetLocationSequenceComponent
| OnLabwareOffsetLocationSequenceComponent
export type LabwareOffsetLocationSequence = LabwareOffsetLocationSequenceComponent[]

export interface LegacyLabwareOffsetLocation {
slotName: string
moduleModel?: ModuleModel
definitionUri?: string
}
export interface LabwareOffsetCreateData {
export interface LegacyLabwareOffsetCreateData {
definitionUri: string
location: LabwareOffsetLocation
location: LegacyLabwareOffsetLocation
vector: VectorOffset
}

Expand Down
2 changes: 1 addition & 1 deletion api/src/opentrons/protocol_api/core/engine/labware.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def set_calibration(self, delta: Point) -> None:

request = LabwareOffsetCreate.model_construct(
definitionUri=self.get_uri(),
location=offset_location,
locationSequence=offset_location,
vector=LabwareOffsetVector(x=delta.x, y=delta.y, z=delta.z),
)
self._engine_client.add_labware_offset(request)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@
from typing import Optional

from opentrons.hardware_control.modules import ModuleModel as HardwareModuleModel
from opentrons.protocol_engine import ProtocolEngine, LabwareOffsetLocation, ModuleModel
from opentrons.protocol_engine import (
ProtocolEngine,
LegacyLabwareOffsetLocation,
ModuleModel,
)
from opentrons.types import DeckSlotName, Point

from ..labware import LabwareLoadParams
Expand Down Expand Up @@ -81,9 +85,9 @@ def find(
See the parent class for param details.
"""
offset = self._labware_view.find_applicable_labware_offset(
offset = self._labware_view.find_applicable_labware_offset_by_legacy_location(
definition_uri=load_params.as_uri(),
location=LabwareOffsetLocation(
location=LegacyLabwareOffsetLocation(
slotName=deck_slot,
moduleModel=(
None
Expand Down
6 changes: 4 additions & 2 deletions api/src/opentrons/protocol_engine/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@

from .types import (
LabwareOffset,
LegacyLabwareOffsetCreate,
LabwareOffsetCreate,
LabwareOffsetVector,
LabwareOffsetLocation,
LegacyLabwareOffsetLocation,
LabwareMovementStrategy,
AddressableOffsetVector,
DeckPoint,
Expand Down Expand Up @@ -96,7 +97,8 @@
"LabwareOffset",
"LabwareOffsetCreate",
"LabwareOffsetVector",
"LabwareOffsetLocation",
"LegacyLabwareOffsetCreate",
"LegacyLabwareOffsetLocation",
"LabwareMovementStrategy",
"AddressableOffsetVector",
"DeckSlotLocation",
Expand Down
4 changes: 2 additions & 2 deletions api/src/opentrons/protocol_engine/actions/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from ..notes.notes import CommandNote
from ..state.update_types import StateUpdate
from ..types import (
LabwareOffsetCreate,
LabwareOffsetCreateInternal,
ModuleDefinition,
Liquid,
DeckConfigurationType,
Expand Down Expand Up @@ -206,7 +206,7 @@ class AddLabwareOffsetAction:

labware_offset_id: str
created_at: datetime
request: LabwareOffsetCreate
request: LabwareOffsetCreateInternal


@dataclasses.dataclass(frozen=True)
Expand Down
2 changes: 2 additions & 0 deletions api/src/opentrons/protocol_engine/errors/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
InvalidLiquidError,
LiquidClassDoesNotExistError,
LiquidClassRedefinitionError,
OffsetLocationInvalidError,
)

from .error_occurrence import ErrorOccurrence, ProtocolCommandFailedError
Expand Down Expand Up @@ -160,6 +161,7 @@
"LocationIsLidDockSlotError",
"InvalidAxisForRobotType",
"NotSupportedOnRobotType",
"OffsetLocationInvalidError",
# error occurrence models
"ErrorOccurrence",
"CommandNotAllowedError",
Expand Down
13 changes: 13 additions & 0 deletions api/src/opentrons/protocol_engine/errors/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,19 @@ def __init__(
super().__init__(ErrorCodes.GENERAL_ERROR, message, details, wrapping)


class OffsetLocationInvalidError(ProtocolEngineError):
"""Raised when encountering an invalid labware offset location sequence."""

def __init__(
self,
message: Optional[str] = None,
details: Optional[Dict[str, Any]] = None,
wrapping: Optional[Sequence[EnumeratedError]] = None,
) -> None:
"""Build an OffsetLocationSequenceDoesNotTerminateAtAnAddressableAreaError."""
super().__init__(ErrorCodes.GENERAL_ERROR, message, details, wrapping)


class SlotDoesNotExistError(ProtocolEngineError):
"""Raised when referencing a deck slot that does not exist."""

Expand Down
73 changes: 3 additions & 70 deletions api/src/opentrons/protocol_engine/execution/equipment.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Equipment command side-effect logic."""

from dataclasses import dataclass
from typing import Optional, overload, Union, List

Expand Down Expand Up @@ -42,10 +43,7 @@
from ..types import (
LabwareLocation,
DeckSlotLocation,
ModuleLocation,
OnLabwareLocation,
LabwareOffset,
LabwareOffsetLocation,
ModuleModel,
ModuleDefinition,
AddressableAreaLocation,
Expand Down Expand Up @@ -633,8 +631,9 @@ def find_applicable_labware_offset_id(
or None if no labware offset will apply.
"""
labware_offset_location = (
self._get_labware_offset_location_from_labware_location(labware_location)
self._state_store.geometry.get_projected_offset_location(labware_location)
)

if labware_offset_location is None:
# No offset for off-deck location.
# Returning None instead of raising an exception allows loading a labware
Expand All @@ -647,72 +646,6 @@ def find_applicable_labware_offset_id(
)
return self._get_id_from_offset(offset)

def _get_labware_offset_location_from_labware_location(
self, labware_location: LabwareLocation
) -> Optional[LabwareOffsetLocation]:
if isinstance(labware_location, DeckSlotLocation):
return LabwareOffsetLocation(slotName=labware_location.slotName)
elif isinstance(labware_location, ModuleLocation):
module_id = labware_location.moduleId
# Allow ModuleNotLoadedError to propagate.
# Note also that we match based on the module's requested model, not its
# actual model, to implement robot-server's documented HTTP API semantics.
module_model = self._state_store.modules.get_requested_model(
module_id=module_id
)

# If `module_model is None`, it probably means that this module was added by
# `ProtocolEngine.use_attached_modules()`, instead of an explicit
# `loadModule` command.
#
# This assert should never raise in practice because:
# 1. `ProtocolEngine.use_attached_modules()` is only used by
# robot-server's "stateless command" endpoints, under `/commands`.
# 2. Those endpoints don't support loading labware, so this code will
# never run.
#
# Nevertheless, if it does happen somehow, we do NOT want to pass the
# `None` value along to `LabwareView.find_applicable_labware_offset()`.
# `None` means something different there, which will cause us to return
# wrong results.
assert module_model is not None, (
"Can't find offsets for labware"
" that are loaded on modules"
" that were loaded with ProtocolEngine.use_attached_modules()."
)

module_location = self._state_store.modules.get_location(
module_id=module_id
)
slot_name = module_location.slotName
return LabwareOffsetLocation(slotName=slot_name, moduleModel=module_model)
elif isinstance(labware_location, OnLabwareLocation):
parent_labware_id = labware_location.labwareId
parent_labware_uri = self._state_store.labware.get_definition_uri(
parent_labware_id
)

base_location = self._state_store.labware.get_parent_location(
parent_labware_id
)
base_labware_offset_location = (
self._get_labware_offset_location_from_labware_location(base_location)
)
if base_labware_offset_location is None:
# No offset for labware sitting on labware off-deck
return None

# If labware is being stacked on itself, all labware in the stack will share a labware offset due to
# them sharing the same definitionUri in `LabwareOffsetLocation`. This will not be true for the
# bottom-most labware, which will have a `DeckSlotLocation` and have its definitionUri field empty.
return LabwareOffsetLocation(
slotName=base_labware_offset_location.slotName,
moduleModel=base_labware_offset_location.moduleModel,
definitionUri=parent_labware_uri,
)
else: # Off deck
return None

@staticmethod
def _get_id_from_offset(labware_offset: Optional[LabwareOffset]) -> Optional[str]:
return None if labware_offset is None else labware_offset.id
Loading

0 comments on commit 5303a3b

Please sign in to comment.