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

feat(api): add getNexTip protocol engine command #17038

Merged
merged 7 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
14 changes: 14 additions & 0 deletions api/src/opentrons/protocol_engine/commands/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,14 @@
VerifyTipPresenceCommandType,
)

from .get_next_tip import (
GetNextTip,
GetNextTipCreate,
GetNextTipParams,
GetNextTipResult,
GetNextTipCommandType,
)

from .liquid_probe import (
LiquidProbe,
LiquidProbeParams,
Expand Down Expand Up @@ -595,6 +603,12 @@
"VerifyTipPresenceParams",
"VerifyTipPresenceResult",
"VerifyTipPresenceCommandType",
# get next tip command bundle
"GetNextTip",
"GetNextTipCreate",
"GetNextTipParams",
"GetNextTipResult",
"GetNextTipCommandType",
# liquid probe command bundle
"LiquidProbe",
"LiquidProbeParams",
Expand Down
13 changes: 13 additions & 0 deletions api/src/opentrons/protocol_engine/commands/command_unions.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,14 @@
GetTipPresenceCommandType,
)

from .get_next_tip import (
GetNextTip,
GetNextTipCreate,
GetNextTipParams,
GetNextTipResult,
GetNextTipCommandType,
)

from .liquid_probe import (
LiquidProbe,
LiquidProbeParams,
Expand Down Expand Up @@ -375,6 +383,7 @@
SetStatusBar,
VerifyTipPresence,
GetTipPresence,
GetNextTip,
LiquidProbe,
TryLiquidProbe,
heater_shaker.WaitForTemperature,
Expand Down Expand Up @@ -460,6 +469,7 @@
SetStatusBarParams,
VerifyTipPresenceParams,
GetTipPresenceParams,
GetNextTipParams,
LiquidProbeParams,
TryLiquidProbeParams,
heater_shaker.WaitForTemperatureParams,
Expand Down Expand Up @@ -543,6 +553,7 @@
SetStatusBarCommandType,
VerifyTipPresenceCommandType,
GetTipPresenceCommandType,
GetNextTipCommandType,
LiquidProbeCommandType,
TryLiquidProbeCommandType,
heater_shaker.WaitForTemperatureCommandType,
Expand Down Expand Up @@ -627,6 +638,7 @@
SetStatusBarCreate,
VerifyTipPresenceCreate,
GetTipPresenceCreate,
GetNextTipCreate,
LiquidProbeCreate,
TryLiquidProbeCreate,
heater_shaker.WaitForTemperatureCreate,
Expand Down Expand Up @@ -712,6 +724,7 @@
SetStatusBarResult,
VerifyTipPresenceResult,
GetTipPresenceResult,
GetNextTipResult,
LiquidProbeResult,
TryLiquidProbeResult,
heater_shaker.WaitForTemperatureResult,
Expand Down
123 changes: 123 additions & 0 deletions api/src/opentrons/protocol_engine/commands/get_next_tip.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
"""Get next tip command request, result, and implementation models."""

from __future__ import annotations
from pydantic import BaseModel, Field
from typing import TYPE_CHECKING, Optional, Type, List, Literal, Union

from opentrons.types import NozzleConfigurationType

from ..errors import ErrorOccurrence
from ..types import NextTipInfo, NoTipAvailable, NoTipReason
from .pipetting_common import PipetteIdMixin

from .command import (
AbstractCommandImpl,
BaseCommand,
BaseCommandCreate,
SuccessData,
)

if TYPE_CHECKING:
from ..state.state import StateView


GetNextTipCommandType = Literal["getNextTip"]


class GetNextTipParams(PipetteIdMixin):
"""Payload needed to resolve the next available tip."""

labwareIds: List[str] = Field(
...,
description="Labware ID(s) of tip racks to resolve next available tip(s) from"
" Labware IDs will be resolved sequentially",
)
startingTipWell: Optional[str] = Field(
None,
description="Name of starting tip rack 'well'."
" This only applies to the first tip rack in the list provided in labwareIDs",
)


class GetNextTipResult(BaseModel):
"""Result data from the execution of a GetNextTip."""

nextTipInfo: Union[NextTipInfo, NoTipAvailable] = Field(
...,
description="Labware ID and well name of next available tip for a pipette,"
" or information why no tip could be resolved.",
)


class GetNextTipImplementation(
AbstractCommandImpl[GetNextTipParams, SuccessData[GetNextTipResult]]
):
"""Get next tip command implementation."""

def __init__(
self,
state_view: StateView,
**kwargs: object,
) -> None:
self._state_view = state_view

async def execute(self, params: GetNextTipParams) -> SuccessData[GetNextTipResult]:
"""Get the next available tip for the requested pipette."""
pipette_id = params.pipetteId
starting_tip_name = params.startingTipWell

num_tips = self._state_view.tips.get_pipette_active_channels(pipette_id)
nozzle_map = self._state_view.tips.get_pipette_nozzle_map(pipette_id)

if (
starting_tip_name is not None
and nozzle_map.configuration != NozzleConfigurationType.FULL
Copy link
Contributor

Choose a reason for hiding this comment

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

What does NozzleConfigurationType.FULL mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The pipette is configured to pick up all tips on each channel

):
return SuccessData(
public=GetNextTipResult(
nextTipInfo=NoTipAvailable(
noTipReason=NoTipReason.STARTING_TIP_WITH_PARTIAL,
message="Cannot automatically resolve next tip with starting tip and partial tip configuration.",
Copy link
Contributor

Choose a reason for hiding this comment

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

If this limitation is to match another limitation elsewhere in our codebase, it might be worth a comment here to describe where the underlying limitation comes from, to make things less confusing if the underlying limitation gets fixed.

)
)
)

next_tip: Union[NextTipInfo, NoTipAvailable]
for labware_id in params.labwareIds:
well_name = self._state_view.tips.get_next_tip(
labware_id=labware_id,
num_tips=num_tips,
starting_tip_name=starting_tip_name,
nozzle_map=nozzle_map,
)
if well_name is not None:
next_tip = NextTipInfo(labwareId=labware_id, tipOriginWell=well_name)
break
# After the first tip rack is exhausted, starting tip no longer applies
starting_tip_name = None
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, so this implementation does not try to remember which tiprack has available tips? It just loops through all the tipracks on every call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It'll look at all tip racks given, in order, regardless of if it's completely empty. This is similar to how the API code figures out next tip (it never unassigns a tip rack once its been depleted)

else:
next_tip = NoTipAvailable(
noTipReason=NoTipReason.NO_AVAILABLE_TIPS,
message="No available tips for given pipette, nozzle configuration and provided tip racks.",
)

return SuccessData(public=GetNextTipResult(nextTipInfo=next_tip))
Copy link
Contributor

@SyntaxColoring SyntaxColoring Dec 4, 2024

Choose a reason for hiding this comment

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

This looks like it'll work as described, but if this command isn't actually doing anything—not affecting state nor making the robot move—I guess it doesn't seem like we should be making it a command?

Like, PAPI can get this functionality by calling state_view.tips.get_next_tip(). Or some higher-level function built around that, if it's not sufficient on its own. And the HTTP API can expose this through some non-command endpoint, probably /runs/{runId}/currentState.

Reasons to avoid making it a command include avoiding database compatibility concerns and avoiding increasing our public API surface area. Especially given weirdness about startingWellName, mentioned below.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, ya, that is a good point.

Copy link
Member

Choose a reason for hiding this comment

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

I get what you're saying, but the thing is that we don't have other well-organized means to do a "structured query", I guess you'd say. It's a component of a pickUpNextTip command, or really pickUpNextTip concept, that will need to be in transfers.

We absolutely could implement it as a public method of the engine, or of an engine state view, and plumb it up through the runner and the orchestrator and the run store and the run to an HTTP route, but that increases our public API surface area also. And this way you get to see it in the runlog.

Copy link
Member

@sanni-t sanni-t Dec 4, 2024

Choose a reason for hiding this comment

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

I guess I want to pass the question back at you @SyntaxColoring as to whether error recovery or any other EXEC work will need a client to fetch the next tip from engine. We are planning on adding a pickUpNextTip command sometime later but the critical part for us is knowing which tip to pick up from, and as you correctly gathered, we will be fine using a state getter function for this since it's an internal code that'll need this info. But not sure if a client will need to fetch this info too. Since auto-tip tracking in engine is a feature that is needed by a few projects, a command seemed more useful but maybe we don't need both getNextTip and pickUpNextTip.

Copy link
Contributor

Choose a reason for hiding this comment

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

I get what you're saying, but the thing is that we don't have other well-organized means to do a "structured query", I guess you'd say. It's a component of a pickUpNextTip command, or really pickUpNextTip concept, that will need to be in transfers.

Sorry, I'm not following what you mean by "structured query." I might be missing context. I've seen AUTH-1063 and #16602 but I don't see the part that a separate command helps with. Should I be looking somewhere else?

I guess I want to pass the question back at you as to whether error recovery or any other EXEC work will need a client to fetch the next tip from engine.

Possibly "yes," but if so, I am like 90% sure we will want to implement that as a safe, clean, read-only GET request, instead of running a live command with a POST request and having to contend with stuff like play/pause/door-open run states.



class GetNextTip(BaseCommand[GetNextTipParams, GetNextTipResult, ErrorOccurrence]):
"""Get next tip command model."""

commandType: GetNextTipCommandType = "getNextTip"
params: GetNextTipParams
result: Optional[GetNextTipResult]

_ImplementationCls: Type[GetNextTipImplementation] = GetNextTipImplementation


class GetNextTipCreate(BaseCommandCreate[GetNextTipParams]):
"""Get next tip command creation request model."""

commandType: GetNextTipCommandType = "getNextTip"
params: GetNextTipParams

_CommandCls: Type[GetNextTip] = GetNextTip
31 changes: 31 additions & 0 deletions api/src/opentrons/protocol_engine/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -1115,6 +1115,37 @@ def from_hw_state(cls, state: HwTipStateType) -> "TipPresenceStatus":
}[state]


class NextTipInfo(BaseModel):
"""Next available tip labware and well name data."""

labwareId: str = Field(
...,
description="The labware ID of the tip rack where the next available tip(s) are located.",
)
tipOriginWell: str = Field(
..., description="The (starting) well name of the next available tip(s)."
SyntaxColoring marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Continuing with Max's point, wouldn't 'origin' be yet another term that means the same thing as 'primary' and 'starting' tip? Is it used somewhere already or will it be a new term? Also the description says 'starting well name..'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I came up with this with Casey but yeah, I see your point in muddying the waters with another term. I'll change it to tipStartingWell for consistency

)


class NoTipReason(Enum):
"""The cause of no tip being available for a pipette and tip rack(s)."""

NO_AVAILABLE_TIPS = "noAvailableTips"
STARTING_TIP_WITH_PARTIAL = "startingTipWithPartial"
INCOMPATIBLE_CONFIGURATION = "incompatibleConfiguration"


class NoTipAvailable(BaseModel):
"""No available next tip data."""

noTipReason: NoTipReason = Field(
..., description="The reason why no next available tip could be provided."
)
message: Optional[str] = Field(
None, description="Optional message explaining why a tip wasn't available."
)


# TODO (spp, 2024-04-02): move all RTP types to runner
class RTPBase(BaseModel):
"""Parameters defined in a protocol."""
Expand Down
142 changes: 142 additions & 0 deletions api/tests/opentrons/protocol_engine/commands/test_get_next_tip.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
"""Test get next tip in place commands."""
from decoy import Decoy

from opentrons.types import NozzleConfigurationType
from opentrons.protocol_engine import StateView
from opentrons.protocol_engine.types import NextTipInfo, NoTipAvailable, NoTipReason
from opentrons.protocol_engine.commands.command import SuccessData
from opentrons.protocol_engine.commands.get_next_tip import (
GetNextTipParams,
GetNextTipResult,
GetNextTipImplementation,
)

from opentrons.hardware_control.nozzle_manager import NozzleMap


async def test_get_next_tip_implementation(
decoy: Decoy,
state_view: StateView,
) -> None:
"""A GetNextTip command should have an execution implementation."""
subject = GetNextTipImplementation(state_view=state_view)
params = GetNextTipParams(
pipetteId="abc", labwareIds=["123"], startingTipWell="xyz"
)
mock_nozzle_map = decoy.mock(cls=NozzleMap)

decoy.when(state_view.tips.get_pipette_active_channels("abc")).then_return(42)
decoy.when(state_view.tips.get_pipette_nozzle_map("abc")).then_return(
mock_nozzle_map
)
decoy.when(mock_nozzle_map.configuration).then_return(NozzleConfigurationType.FULL)

decoy.when(
state_view.tips.get_next_tip(
labware_id="123",
num_tips=42,
Copy link
Contributor

Choose a reason for hiding this comment

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

So in this test, the caller is requesting the next tip for a hypothetical pipette that has 42 tips?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I use that to make it clear that this test is not testing veracity of testing the get_next_tip logic, but is passing the expected values to this function

starting_tip_name="xyz",
nozzle_map=mock_nozzle_map,
)
).then_return("foo")

result = await subject.execute(params)

assert result == SuccessData(
public=GetNextTipResult(
nextTipInfo=NextTipInfo(labwareId="123", tipOriginWell="foo")
),
)


async def test_get_next_tip_implementation_multiple_tip_racks(
decoy: Decoy,
state_view: StateView,
) -> None:
"""A GetNextTip command with multiple tip racks should not apply starting tip to the following ones."""
subject = GetNextTipImplementation(state_view=state_view)
params = GetNextTipParams(
pipetteId="abc", labwareIds=["123", "456"], startingTipWell="xyz"
)
mock_nozzle_map = decoy.mock(cls=NozzleMap)

decoy.when(state_view.tips.get_pipette_active_channels("abc")).then_return(42)
decoy.when(state_view.tips.get_pipette_nozzle_map("abc")).then_return(
mock_nozzle_map
)
decoy.when(mock_nozzle_map.configuration).then_return(NozzleConfigurationType.FULL)

decoy.when(
state_view.tips.get_next_tip(
labware_id="456",
num_tips=42,
starting_tip_name=None,
nozzle_map=mock_nozzle_map,
)
).then_return("foo")
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, it might be nice if this test could demonstrate the code working when there are multiple labwares and the first one doesn't have any tips left, because I'm kind of curious how your for-loop in the implementation would work.


result = await subject.execute(params)

assert result == SuccessData(
public=GetNextTipResult(
nextTipInfo=NextTipInfo(labwareId="456", tipOriginWell="foo")
),
)


async def test_get_next_tip_implementation_no_tips(
decoy: Decoy,
state_view: StateView,
) -> None:
"""A GetNextTip command should return with NoTipAvailable if there are no available tips."""
subject = GetNextTipImplementation(state_view=state_view)
params = GetNextTipParams(
pipetteId="abc", labwareIds=["123", "456"], startingTipWell="xyz"
)
mock_nozzle_map = decoy.mock(cls=NozzleMap)

decoy.when(state_view.tips.get_pipette_active_channels("abc")).then_return(42)
decoy.when(state_view.tips.get_pipette_nozzle_map("abc")).then_return(
mock_nozzle_map
)
decoy.when(mock_nozzle_map.configuration).then_return(NozzleConfigurationType.FULL)

result = await subject.execute(params)

assert result == SuccessData(
public=GetNextTipResult(
nextTipInfo=NoTipAvailable(
noTipReason=NoTipReason.NO_AVAILABLE_TIPS,
message="No available tips for given pipette, nozzle configuration and provided tip racks.",
)
),
)


async def test_get_next_tip_implementation_partial_with_starting_tip(
decoy: Decoy,
state_view: StateView,
) -> None:
"""A GetNextTip command should return with NoTipAvailable if there's a starting tip and a partial config."""
subject = GetNextTipImplementation(state_view=state_view)
params = GetNextTipParams(
pipetteId="abc", labwareIds=["123", "456"], startingTipWell="xyz"
)
mock_nozzle_map = decoy.mock(cls=NozzleMap)

decoy.when(state_view.tips.get_pipette_active_channels("abc")).then_return(42)
decoy.when(state_view.tips.get_pipette_nozzle_map("abc")).then_return(
mock_nozzle_map
)
decoy.when(mock_nozzle_map.configuration).then_return(NozzleConfigurationType.ROW)

result = await subject.execute(params)

assert result == SuccessData(
public=GetNextTipResult(
nextTipInfo=NoTipAvailable(
noTipReason=NoTipReason.STARTING_TIP_WITH_PARTIAL,
message="Cannot automatically resolve next tip with starting tip and partial tip configuration.",
)
),
)
Loading
Loading