-
Notifications
You must be signed in to change notification settings - Fork 178
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(api): Allow treating errors as false-positives (ignore them and …
…continue with the run) (#16556)
- Loading branch information
1 parent
288b8a4
commit 24fcc0d
Showing
36 changed files
with
560 additions
and
148 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
29 changes: 23 additions & 6 deletions
29
api/src/opentrons/protocol_engine/actions/get_state_update.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,35 @@ | ||
# noqa: D100 | ||
|
||
|
||
from .actions import Action, SucceedCommandAction, FailCommandAction | ||
from .actions import ( | ||
Action, | ||
ResumeFromRecoveryAction, | ||
SucceedCommandAction, | ||
FailCommandAction, | ||
) | ||
from ..commands.command import DefinedErrorData | ||
from ..error_recovery_policy import ErrorRecoveryType | ||
from ..state.update_types import StateUpdate | ||
|
||
|
||
def get_state_update(action: Action) -> StateUpdate | None: | ||
"""Extract the StateUpdate from an action, if there is one.""" | ||
def get_state_updates(action: Action) -> list[StateUpdate]: | ||
"""Extract all the StateUpdates that the StateStores should apply when they apply an action.""" | ||
if isinstance(action, SucceedCommandAction): | ||
return action.state_update | ||
return [action.state_update] | ||
|
||
elif isinstance(action, FailCommandAction) and isinstance( | ||
action.error, DefinedErrorData | ||
): | ||
return action.error.state_update | ||
if action.type == ErrorRecoveryType.ASSUME_FALSE_POSITIVE_AND_CONTINUE: | ||
return [ | ||
action.error.state_update, | ||
action.error.state_update_if_false_positive, | ||
] | ||
else: | ||
return [action.error.state_update] | ||
|
||
elif isinstance(action, ResumeFromRecoveryAction): | ||
return [action.state_update] | ||
|
||
else: | ||
return None | ||
return [] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
101 changes: 101 additions & 0 deletions
101
api/src/opentrons/protocol_engine/execution/error_recovery_hardware_state_synchronizer.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
# noqa: D100 | ||
|
||
|
||
from opentrons.hardware_control import HardwareControlAPI | ||
from opentrons.protocol_engine.actions.action_handler import ActionHandler | ||
from opentrons.protocol_engine.actions.actions import ( | ||
Action, | ||
FailCommandAction, | ||
ResumeFromRecoveryAction, | ||
) | ||
from opentrons.protocol_engine.commands.command import DefinedErrorData | ||
from opentrons.protocol_engine.error_recovery_policy import ErrorRecoveryType | ||
from opentrons.protocol_engine.execution.tip_handler import HardwareTipHandler | ||
from opentrons.protocol_engine.state import update_types | ||
from opentrons.protocol_engine.state.state import StateView | ||
|
||
|
||
class ErrorRecoveryHardwareStateSynchronizer(ActionHandler): | ||
"""A hack to keep the hardware API's state correct through certain error recovery flows. | ||
BACKGROUND: | ||
Certain parts of robot state are duplicated between `opentrons.protocol_engine` and | ||
`opentrons.hardware_control`. Stuff like "is there a tip attached." | ||
Normally, Protocol Engine command implementations (`opentrons.protocol_engine.commands`) | ||
mutate hardware API state when they execute; and then when they finish executing, | ||
the Protocol Engine state stores (`opentrons.protocol_engine.state`) update Protocol | ||
Engine state accordingly. So both halves are accounted for. This generally works fine. | ||
However, we need to go out of our way to support | ||
`ProtocolEngine.resume_from_recovery(reconcile_false_positive=True)`. | ||
It wants to apply a second set of state updates to "fix things up" with the | ||
new knowledge that some error was a false positive. The Protocol Engine half of that | ||
is easy for us to apply the normal way, through the state stores; but the | ||
hardware API half of that cannot be applied the normal way, from the command | ||
implementation, because the command in question is no longer running. | ||
THE HACK: | ||
This listens for the same error recovery state updates that the state stores do, | ||
figures out what hardware API state mutations ought to go along with them, | ||
and then does those mutations. | ||
The problem is that hardware API state is now mutated from two different places | ||
(sometimes the command implementations, and sometimes here), which are bound | ||
to grow accidental differences. | ||
TO FIX: | ||
Make Protocol Engine's use of the hardware API less stateful. e.g. supply | ||
tip geometry every time we call a hardware API movement method, instead of | ||
just once when we pick up a tip. Use Protocol Engine state as the single source | ||
of truth. | ||
""" | ||
|
||
def __init__(self, hardware_api: HardwareControlAPI, state_view: StateView) -> None: | ||
self._hardware_api = hardware_api | ||
self._state_view = state_view | ||
|
||
def handle_action(self, action: Action) -> None: | ||
"""Modify hardware API state in reaction to a Protocol Engine action.""" | ||
state_update = _get_state_update(action) | ||
if state_update: | ||
self._synchronize(state_update) | ||
|
||
def _synchronize(self, state_update: update_types.StateUpdate) -> None: | ||
tip_handler = HardwareTipHandler(self._state_view, self._hardware_api) | ||
|
||
if state_update.pipette_tip_state != update_types.NO_CHANGE: | ||
pipette_id = state_update.pipette_tip_state.pipette_id | ||
tip_geometry = state_update.pipette_tip_state.tip_geometry | ||
if tip_geometry is None: | ||
tip_handler.remove_tip(pipette_id) | ||
else: | ||
tip_handler.cache_tip(pipette_id=pipette_id, tip=tip_geometry) | ||
|
||
|
||
def _get_state_update(action: Action) -> update_types.StateUpdate | None: | ||
"""Get the mutations that we need to do on the hardware API to stay in sync with an engine action. | ||
The mutations are returned in Protocol Engine terms, as a StateUpdate. | ||
They then need to be converted to hardware API terms. | ||
""" | ||
match action: | ||
case ResumeFromRecoveryAction(state_update=state_update): | ||
return state_update | ||
|
||
case FailCommandAction( | ||
error=DefinedErrorData( | ||
state_update_if_false_positive=state_update_if_false_positive | ||
) | ||
): | ||
return ( | ||
state_update_if_false_positive | ||
if action.type == ErrorRecoveryType.ASSUME_FALSE_POSITIVE_AND_CONTINUE | ||
else None | ||
) | ||
|
||
case _: | ||
return None |
Oops, something went wrong.