From fa83dd3b0ad4c1aa902ff11e8d5f15a10a81373c Mon Sep 17 00:00:00 2001 From: Ed Cormany Date: Thu, 14 Dec 2023 10:27:20 -0500 Subject: [PATCH 01/32] chore: add direct Ethernet feature to release notes (#14199) --- api/release-notes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/api/release-notes.md b/api/release-notes.md index 89be24d892c..8f991daadaa 100644 --- a/api/release-notes.md +++ b/api/release-notes.md @@ -18,6 +18,7 @@ Welcome to the v7.1.0 release of the Opentrons robot software! This release incl ### Improved Features +- The Ethernet port on Flex now supports direct connection to a computer. - Improves aspirate, dispense, and mix behavior with volumes set to zero. - The `opentrons_simulate` command-line tool now works with all Python API versions. From ae8f6377a8216a39a2c44f2dd42e076182240871 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Thu, 14 Dec 2023 12:07:01 -0500 Subject: [PATCH 02/32] feat(api): Allow omitting the mount when loading a 96-channel (#14187) Co-authored-by: Ed Cormany --- api/docs/v2/new_pipette.rst | 6 +- .../protocol_api/protocol_context.py | 25 ++++--- api/src/opentrons/protocol_api/validation.py | 23 +++++- .../protocol_api/test_protocol_context.py | 71 ++++++++----------- .../opentrons/protocol_api/test_validation.py | 45 +++++++++--- 5 files changed, 101 insertions(+), 69 deletions(-) diff --git a/api/docs/v2/new_pipette.rst b/api/docs/v2/new_pipette.rst index 9fafc2e5c95..7d8602b064b 100644 --- a/api/docs/v2/new_pipette.rst +++ b/api/docs/v2/new_pipette.rst @@ -48,13 +48,13 @@ If you're writing a protocol that uses the Flex Gripper, you might think that th Loading a Flex 96-Channel Pipette --------------------------------- -This code sample loads the Flex 96-Channel Pipette. Because of its size, the Flex 96-Channel Pipette requires the left *and* right pipette mounts. You cannot use this pipette with 1- or 8-Channel Pipette in the same protocol or when these instruments are attached to the robot. To load the 96-Channel Pipette, specify its position as ``mount='left'`` as shown here: +This code sample loads the Flex 96-Channel Pipette. Because of its size, the Flex 96-Channel Pipette requires the left *and* right pipette mounts. You cannot use this pipette with 1- or 8-Channel Pipette in the same protocol or when these instruments are attached to the robot. When loading the 96-Channel Pipette, you can omit the ``mount`` argument from ``load_instrument()`` as shown here: .. code-block:: python def run(protocol: protocol_api.ProtocolContext): - left = protocol.load_instrument( - instrument_name='flex_96channel_1000', mount='left') + pipette = protocol.load_instrument( + instrument_name='flex_96channel_1000') .. versionadded:: 2.15 diff --git a/api/src/opentrons/protocol_api/protocol_context.py b/api/src/opentrons/protocol_api/protocol_context.py index ece841fd619..33a2e0b07de 100644 --- a/api/src/opentrons/protocol_api/protocol_context.py +++ b/api/src/opentrons/protocol_api/protocol_context.py @@ -821,7 +821,7 @@ def loaded_modules(self) -> Dict[int, ModuleTypes]: def load_instrument( self, instrument_name: str, - mount: Union[Mount, str], + mount: Union[Mount, str, None] = None, tip_racks: Optional[List[Labware]] = None, replace: bool = False, ) -> InstrumentContext: @@ -831,15 +831,16 @@ def load_instrument( ensure that the correct instrument is attached in the specified location. - :param str instrument_name: The name of the instrument model, or a - prefix. For instance, 'p10_single' may be - used to request a P10 single regardless of - the version. - :param mount: The mount in which this instrument should be attached. + :param str instrument_name: Which instrument you want to load. See :ref:`new-pipette-models` + for the valid values. + :param mount: The mount where this instrument should be attached. This can either be an instance of the enum type - :py:class:`.types.Mount` or one of the strings `'left'` - and `'right'`. - :type mount: types.Mount or str + :py:class:`.types.Mount` or one of the strings ``"left"`` + or ``"right"``. If you're loading a Flex 96-Channel Pipette + (``instrument_name="flex_96channel_1000"``), you can leave this unspecified, + since it always occupies both mounts; if you do specify a value, it will be + ignored. + :type mount: types.Mount or str or ``None`` :param tip_racks: A list of tip racks from which to pick tips if :py:meth:`.InstrumentContext.pick_up_tip` is called without arguments. @@ -850,9 +851,11 @@ def load_instrument( """ instrument_name = validation.ensure_lowercase_name(instrument_name) checked_instrument_name = validation.ensure_pipette_name(instrument_name) - is_96_channel = checked_instrument_name == PipetteNameType.P1000_96 + checked_mount = validation.ensure_mount_for_pipette( + mount, checked_instrument_name + ) - checked_mount = Mount.LEFT if is_96_channel else validation.ensure_mount(mount) + is_96_channel = checked_instrument_name == PipetteNameType.P1000_96 tip_racks = tip_racks or [] diff --git a/api/src/opentrons/protocol_api/validation.py b/api/src/opentrons/protocol_api/validation.py index c4849950c50..372913ad20e 100644 --- a/api/src/opentrons/protocol_api/validation.py +++ b/api/src/opentrons/protocol_api/validation.py @@ -88,7 +88,27 @@ class InvalidTrashBinLocationError(ValueError): """An error raised when attempting to load trash bins in invalid slots.""" -def ensure_mount(mount: Union[str, Mount]) -> Mount: +def ensure_mount_for_pipette( + mount: Union[str, Mount, None], pipette: PipetteNameType +) -> Mount: + """Ensure that an input value represents a valid mount, and is valid for the given pipette.""" + if pipette == PipetteNameType.P1000_96: + # Always validate the raw mount input, even if the pipette is a 96-channel and we're not going + # to use the mount value. + if mount is not None: + _ensure_mount(mount) + # Internal layers treat the 96-channel as being on the left mount. + return Mount.LEFT + else: + if mount is None: + raise InvalidPipetteMountError( + f"You must specify a left or right mount to load {pipette.value}." + ) + else: + return _ensure_mount(mount) + + +def _ensure_mount(mount: Union[str, Mount]) -> Mount: """Ensure that an input value represents a valid Mount.""" if mount in [Mount.EXTENSION, "extension"]: # This would cause existing protocols that might be iterating over mount types @@ -274,7 +294,6 @@ def ensure_module_model(load_name: str) -> ModuleModel: def ensure_and_convert_trash_bin_location( deck_slot: Union[int, str], api_version: APIVersion, robot_type: RobotType ) -> str: - """Ensure trash bin load location is valid. Also, convert the deck slot to a valid trash bin addressable area. diff --git a/api/tests/opentrons/protocol_api/test_protocol_context.py b/api/tests/opentrons/protocol_api/test_protocol_context.py index 19a1abd202a..fd3c8000664 100644 --- a/api/tests/opentrons/protocol_api/test_protocol_context.py +++ b/api/tests/opentrons/protocol_api/test_protocol_context.py @@ -150,11 +150,15 @@ def test_load_instrument( mock_instrument_core = decoy.mock(cls=InstrumentCore) mock_tip_racks = [decoy.mock(cls=Labware), decoy.mock(cls=Labware)] - decoy.when(mock_validation.ensure_mount("shadowfax")).then_return(Mount.LEFT) decoy.when(mock_validation.ensure_lowercase_name("Gandalf")).then_return("gandalf") decoy.when(mock_validation.ensure_pipette_name("gandalf")).then_return( PipetteNameType.P300_SINGLE ) + decoy.when( + mock_validation.ensure_mount_for_pipette( + "shadowfax", PipetteNameType.P300_SINGLE + ) + ).then_return(Mount.LEFT) decoy.when( mock_core.load_instrument( @@ -197,13 +201,17 @@ def test_load_instrument_replace( """It should allow/disallow pipette replacement.""" mock_instrument_core = decoy.mock(cls=InstrumentCore) - decoy.when(mock_validation.ensure_lowercase_name("ada")).then_return("ada") - decoy.when(mock_validation.ensure_mount(matchers.IsA(Mount))).then_return( - Mount.RIGHT + decoy.when(mock_validation.ensure_lowercase_name(matchers.IsA(str))).then_return( + "ada" ) decoy.when(mock_validation.ensure_pipette_name(matchers.IsA(str))).then_return( PipetteNameType.P300_SINGLE ) + decoy.when( + mock_validation.ensure_mount_for_pipette( + matchers.IsA(Mount), matchers.IsA(PipetteNameType) + ) + ).then_return(Mount.RIGHT) decoy.when( mock_core.load_instrument( instrument_name=matchers.IsA(PipetteNameType), @@ -227,36 +235,6 @@ def test_load_instrument_replace( subject.load_instrument(instrument_name="ada", mount=Mount.RIGHT) -def test_96_channel_pipette_always_loads_on_the_left_mount( - decoy: Decoy, - mock_core: ProtocolCore, - subject: ProtocolContext, -) -> None: - """It should always load a 96-channel pipette on left mount, regardless of the mount arg specified.""" - mock_instrument_core = decoy.mock(cls=InstrumentCore) - - decoy.when(mock_validation.ensure_lowercase_name("A 96 Channel Name")).then_return( - "a 96 channel name" - ) - decoy.when(mock_validation.ensure_pipette_name("a 96 channel name")).then_return( - PipetteNameType.P1000_96 - ) - decoy.when( - mock_core.load_instrument( - instrument_name=PipetteNameType.P1000_96, - mount=Mount.LEFT, - ) - ).then_return(mock_instrument_core) - decoy.when(mock_core.get_disposal_locations()).then_raise( - NoTrashDefinedError("No trash!") - ) - - result = subject.load_instrument( - instrument_name="A 96 Channel Name", mount="shadowfax" - ) - assert result == subject.loaded_instruments["left"] - - def test_96_channel_pipette_raises_if_another_pipette_attached( decoy: Decoy, mock_core: ProtocolCore, @@ -265,13 +243,17 @@ def test_96_channel_pipette_raises_if_another_pipette_attached( """It should always raise when loading a 96-channel pipette when another pipette is attached.""" mock_instrument_core = decoy.mock(cls=InstrumentCore) - decoy.when(mock_validation.ensure_lowercase_name("ada")).then_return("ada") - decoy.when(mock_validation.ensure_pipette_name("ada")).then_return( - PipetteNameType.P300_SINGLE - ) - decoy.when(mock_validation.ensure_mount(matchers.IsA(Mount))).then_return( - Mount.RIGHT - ) + decoy.when( + mock_validation.ensure_lowercase_name("A Single Channel Name") + ).then_return("a single channel name") + decoy.when( + mock_validation.ensure_pipette_name("a single channel name") + ).then_return(PipetteNameType.P300_SINGLE) + decoy.when( + mock_validation.ensure_mount_for_pipette( + Mount.RIGHT, PipetteNameType.P300_SINGLE + ) + ).then_return(Mount.RIGHT) decoy.when( mock_core.load_instrument( @@ -286,7 +268,9 @@ def test_96_channel_pipette_raises_if_another_pipette_attached( NoTrashDefinedError("No trash!") ) - pipette_1 = subject.load_instrument(instrument_name="ada", mount=Mount.RIGHT) + pipette_1 = subject.load_instrument( + instrument_name="A Single Channel Name", mount=Mount.RIGHT + ) assert subject.loaded_instruments["right"] is pipette_1 decoy.when(mock_validation.ensure_lowercase_name("A 96 Channel Name")).then_return( @@ -295,6 +279,9 @@ def test_96_channel_pipette_raises_if_another_pipette_attached( decoy.when(mock_validation.ensure_pipette_name("a 96 channel name")).then_return( PipetteNameType.P1000_96 ) + decoy.when( + mock_validation.ensure_mount_for_pipette("shadowfax", PipetteNameType.P1000_96) + ).then_return(Mount.LEFT) decoy.when( mock_core.load_instrument( instrument_name=PipetteNameType.P1000_96, diff --git a/api/tests/opentrons/protocol_api/test_validation.py b/api/tests/opentrons/protocol_api/test_validation.py index 4d41eb4562d..667349f0f8d 100644 --- a/api/tests/opentrons/protocol_api/test_validation.py +++ b/api/tests/opentrons/protocol_api/test_validation.py @@ -28,18 +28,28 @@ @pytest.mark.parametrize( - ["input_value", "expected"], + ["input_mount", "input_pipette", "expected"], [ - ("left", Mount.LEFT), - ("right", Mount.RIGHT), - ("LeFt", Mount.LEFT), - (Mount.LEFT, Mount.LEFT), - (Mount.RIGHT, Mount.RIGHT), + # Different string capitalizations: + ("left", PipetteNameType.P300_MULTI_GEN2, Mount.LEFT), + ("right", PipetteNameType.P300_MULTI_GEN2, Mount.RIGHT), + ("LeFt", PipetteNameType.P300_MULTI_GEN2, Mount.LEFT), + # Passing in a Mount: + (Mount.LEFT, PipetteNameType.P300_MULTI_GEN2, Mount.LEFT), + (Mount.RIGHT, PipetteNameType.P300_MULTI_GEN2, Mount.RIGHT), + # Special handling for the 96-channel: + ("left", PipetteNameType.P1000_96, Mount.LEFT), + ("right", PipetteNameType.P1000_96, Mount.LEFT), + (None, PipetteNameType.P1000_96, Mount.LEFT), ], ) -def test_ensure_mount(input_value: Union[str, Mount], expected: Mount) -> None: +def test_ensure_mount( + input_mount: Union[str, Mount, None], + input_pipette: PipetteNameType, + expected: Mount, +) -> None: """It should properly map strings and mounts.""" - result = subject.ensure_mount(input_value) + result = subject.ensure_mount_for_pipette(input_mount, input_pipette) assert result == expected @@ -48,18 +58,31 @@ def test_ensure_mount_input_invalid() -> None: with pytest.raises( subject.InvalidPipetteMountError, match="must be 'left' or 'right'" ): - subject.ensure_mount("oh no") + subject.ensure_mount_for_pipette("oh no", PipetteNameType.P300_MULTI_GEN2) + + # Any mount is valid for the 96-Channel, but it needs to be a valid mount. + with pytest.raises( + subject.InvalidPipetteMountError, match="must be 'left' or 'right'" + ): + subject.ensure_mount_for_pipette("oh no", PipetteNameType.P1000_96) with pytest.raises( subject.PipetteMountTypeError, match="'left', 'right', or an opentrons.types.Mount", ): - subject.ensure_mount(42) # type: ignore[arg-type] + subject.ensure_mount_for_pipette(42, PipetteNameType.P300_MULTI_GEN2) # type: ignore[arg-type] with pytest.raises( subject.InvalidPipetteMountError, match="Use the left or right mounts instead" ): - subject.ensure_mount(Mount.EXTENSION) + subject.ensure_mount_for_pipette( + Mount.EXTENSION, PipetteNameType.P300_MULTI_GEN2 + ) + + with pytest.raises( + subject.InvalidPipetteMountError, match="You must specify a left or right mount" + ): + subject.ensure_mount_for_pipette(None, PipetteNameType.P300_MULTI_GEN2) @pytest.mark.parametrize( From 42efa316c50863aca03c1cd2a59f45b140ab1caf Mon Sep 17 00:00:00 2001 From: Ed Cormany Date: Thu, 14 Dec 2023 14:40:18 -0500 Subject: [PATCH 03/32] chore: add probe-based LPC to app release notes (#14202) --- app-shell/build/release-notes.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app-shell/build/release-notes.md b/app-shell/build/release-notes.md index b4b8acafb83..47fc0e4583f 100644 --- a/app-shell/build/release-notes.md +++ b/app-shell/build/release-notes.md @@ -10,13 +10,17 @@ log][]. For a list of currently known issues, please see the [Opentrons issue tr Welcome to the v7.1.0 release of the Opentrons App! This release includes new deck and pipette functionality for Opentrons Flex, a new workflow for dropping tips after a protocol is canceled, and other improvements. -### New features +### New Features - Specify the deck configuration of Flex, including the movable trash bin, waste chute, and staging area slots. - Resolve conflicts between the hardware a protocol requires and the current deck configuration as part of run setup. - Run protocols that use the Flex 96-Channel Pipette, including partial tip pickup. - Choose where to dispense liquid and drop tips held by a pipette when a protocol is canceled. +### Improved Features + +- Labware Position Check on Flex uses the pipette calibration probe, instead of a tip, for greater accuracy. + ### Bug Fixes - Labware Position Check no longer tries to check the same labware in the same position twice, which was leading to errors. From 4ab142a0c99ec4489a48f1907dcc6441ce40661d Mon Sep 17 00:00:00 2001 From: Brent Hagen Date: Thu, 14 Dec 2023 15:05:40 -0500 Subject: [PATCH 04/32] fix(shared-data): add moveToAddressableAreaForDropTip to getAddressableAreasInProtocol (#14205) adds the moveToAddressableAreaForDropTip command to the getAddressableAreasInProtocol helper that provides a list of addressable area names referenced in a protocol's commands --- shared-data/js/helpers/getAddressableAreasInProtocol.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/shared-data/js/helpers/getAddressableAreasInProtocol.ts b/shared-data/js/helpers/getAddressableAreasInProtocol.ts index fae6fc0d276..66ae4287c32 100644 --- a/shared-data/js/helpers/getAddressableAreasInProtocol.ts +++ b/shared-data/js/helpers/getAddressableAreasInProtocol.ts @@ -75,6 +75,14 @@ export function getAddressableAreasInProtocol( ...acc, command.params.addressableAreaName as AddressableAreaName, ] + } else if ( + command.commandType === 'moveToAddressableAreaForDropTip' && + !acc.includes(command.params.addressableAreaName as AddressableAreaName) + ) { + return [ + ...acc, + command.params.addressableAreaName as AddressableAreaName, + ] } else { return acc } From a7d11875129ed340068318c495e4dc1325f46a19 Mon Sep 17 00:00:00 2001 From: Jamey H Date: Thu, 14 Dec 2023 17:19:29 -0500 Subject: [PATCH 05/32] fix(app): fix various drop tip wizard issues (#14207) This PR is a general second-pass of the drop tip wizard. There's a good bit refactoring, but in terms of functional changes: - The wizard doesn't render BeforeBeginning until the maintenance run has been successfully created - renders a spinner now. This prevents some bugs and is better UX. - Exiting the drop tip wizard now renders a spinner and exits properly. - The home command occurs in a predictable spot - no more instantly homing after completing blowout. - Use moveToAddressableArea instead of moveRelative to prevent unexpected pipette movements (you CANNOT move to a slot with a moveable trash or waste chute loaded -- this is a known issue and currently being fixed). - Fixes an issue where an unreachable step could be hit. - More error messaging! NOTE: you CANNOT move to a slot with a moveable trash or waste chute loaded -- this is a known issue and currently being addressed separately. Closes RQA-2083, RQA-2094, RQA-2066, RQA-2068, RQA-2024 --- .../DropTipWizard/BeforeBeginning.tsx | 36 ++-- .../DropTipWizard/ChooseLocation.tsx | 33 +--- .../DropTipWizard/ExitConfirmation.tsx | 8 +- .../organisms/DropTipWizard/JogToPosition.tsx | 23 ++- app/src/organisms/DropTipWizard/Success.tsx | 21 +-- app/src/organisms/DropTipWizard/index.tsx | 159 +++++++++--------- .../InstrumentDetailOverflowMenu.test.tsx | 4 +- 7 files changed, 130 insertions(+), 154 deletions(-) diff --git a/app/src/organisms/DropTipWizard/BeforeBeginning.tsx b/app/src/organisms/DropTipWizard/BeforeBeginning.tsx index f9a740283e7..0333827ab54 100644 --- a/app/src/organisms/DropTipWizard/BeforeBeginning.tsx +++ b/app/src/organisms/DropTipWizard/BeforeBeginning.tsx @@ -1,6 +1,7 @@ import * as React from 'react' import styled, { css } from 'styled-components' import { useTranslation } from 'react-i18next' + import { Flex, SPACING, @@ -19,34 +20,23 @@ import { JUSTIFY_FLEX_END, JUSTIFY_SPACE_AROUND, } from '@opentrons/components' + import { StyledText } from '../../atoms/text' import { SmallButton, MediumButton } from '../../atoms/buttons' +import { InProgressModal } from '../../molecules/InProgressModal/InProgressModal' // import { NeedHelpLink } from '../CalibrationPanels' import blowoutVideo from '../../assets/videos/droptip-wizard/Blowout-Liquid.webm' import droptipVideo from '../../assets/videos/droptip-wizard/Drop-tip.webm' -import type { UseMutateFunction } from 'react-query' -import type { AxiosError } from 'axios' -import type { - CreateMaintenanceRunData, - MaintenanceRun, -} from '@opentrons/api-client' - // TODO: get help link article URL // const NEED_HELP_URL = '' interface BeforeBeginningProps { setShouldDispenseLiquid: (shouldDispenseLiquid: boolean) => void - createMaintenanceRun: UseMutateFunction< - MaintenanceRun, - AxiosError, - CreateMaintenanceRunData, - unknown - > createdMaintenanceRunId: string | null - isCreateLoading: boolean isOnDevice: boolean + isRobotMoving: boolean } export const BeforeBeginning = ( @@ -54,10 +44,9 @@ export const BeforeBeginning = ( ): JSX.Element | null => { const { setShouldDispenseLiquid, - createMaintenanceRun, createdMaintenanceRunId, - isCreateLoading, isOnDevice, + isRobotMoving, } = props const { i18n, t } = useTranslation(['drop_tip_wizard', 'shared']) const [flowType, setFlowType] = React.useState< @@ -68,11 +57,9 @@ export const BeforeBeginning = ( setShouldDispenseLiquid(flowType === 'liquid_and_tips') } - React.useEffect(() => { - if (createdMaintenanceRunId == null) { - createMaintenanceRun({}) - } - }, []) + if (isRobotMoving || createdMaintenanceRunId == null) { + return + } if (isOnDevice) { return ( @@ -118,7 +105,7 @@ export const BeforeBeginning = ( @@ -180,10 +167,7 @@ export const BeforeBeginning = ( {/* */} - + {i18n.format(t('shared:continue'), 'capitalize')} diff --git a/app/src/organisms/DropTipWizard/ChooseLocation.tsx b/app/src/organisms/DropTipWizard/ChooseLocation.tsx index 0de5c0557b8..5a384b217ef 100644 --- a/app/src/organisms/DropTipWizard/ChooseLocation.tsx +++ b/app/src/organisms/DropTipWizard/ChooseLocation.tsx @@ -18,10 +18,7 @@ import { SPACING, TYPOGRAPHY, } from '@opentrons/components' -import { - getDeckDefFromRobotType, - getPositionFromSlotId, -} from '@opentrons/shared-data' +import { getDeckDefFromRobotType } from '@opentrons/shared-data' import { SmallButton } from '../../atoms/buttons' import { StyledText } from '../../atoms/text' @@ -41,7 +38,9 @@ interface ChooseLocationProps { title: string body: string | JSX.Element robotType: RobotType - moveToXYCoordinate: (x: number, y: number) => Promise + moveToAddressableArea: ( + addressableArea: string + ) => Promise isRobotMoving: boolean isOnDevice: boolean setErrorMessage: (arg0: string) => void @@ -56,7 +55,7 @@ export const ChooseLocation = ( title, body, robotType, - moveToXYCoordinate, + moveToAddressableArea, isRobotMoving, isOnDevice, setErrorMessage, @@ -70,26 +69,10 @@ export const ChooseLocation = ( const handleConfirmPosition = (): void => { const deckSlot = deckDef.locations.addressableAreas.find( l => l.id === selectedLocation.slotName - ) - - const slotPosition = getPositionFromSlotId( - selectedLocation.slotName, - deckDef - ) + )?.id - const slotX = slotPosition?.[0] - const slotY = slotPosition?.[1] - const xDimension = deckSlot?.boundingBox.xDimension - const yDimension = deckSlot?.boundingBox.yDimension - if ( - slotX != null && - slotY != null && - xDimension != null && - yDimension != null - ) { - const targetX = slotX + xDimension / 2 - const targetY = slotY + yDimension / 2 - moveToXYCoordinate(targetX, targetY) + if (deckSlot != null) { + moveToAddressableArea(deckSlot) .then(() => handleProceed()) .catch(e => setErrorMessage(`${e.message}`)) } diff --git a/app/src/organisms/DropTipWizard/ExitConfirmation.tsx b/app/src/organisms/DropTipWizard/ExitConfirmation.tsx index a0ab8178c61..42f1a6f2d7d 100644 --- a/app/src/organisms/DropTipWizard/ExitConfirmation.tsx +++ b/app/src/organisms/DropTipWizard/ExitConfirmation.tsx @@ -27,9 +27,11 @@ export function ExitConfirmation(props: ExitConfirmationProps): JSX.Element { const flowTitle = t('drop_tips') const isOnDevice = useSelector(getIsOnDevice) - return isRobotMoving ? ( - - ) : ( + if (isRobotMoving) { + return + } + + return ( { + setIsRobotInMotion(() => true) + handleGoBack() + } if (showPositionConfirmation) { - return isRobotMoving ? ( + return isRobotInMotion ? ( ) : ( { + setIsRobotInMotion(true) + handleProceed() + }} handleGoBack={() => setShowPositionConfirmation(false)} isOnDevice={isOnDevice} currentStep={currentStep} @@ -191,6 +201,11 @@ export const JogToPosition = ( ) } + // Moving due to "Exit" or "Go back" click. + if (isRobotInMotion) { + return + } + if (isOnDevice) { return ( @@ -254,7 +269,7 @@ export const JogToPosition = ( > {/* */} - + {t('shared:go_back')} setShowPositionConfirmation(true)}> diff --git a/app/src/organisms/DropTipWizard/Success.tsx b/app/src/organisms/DropTipWizard/Success.tsx index 6afc9327c86..c68563722ab 100644 --- a/app/src/organisms/DropTipWizard/Success.tsx +++ b/app/src/organisms/DropTipWizard/Success.tsx @@ -15,28 +15,19 @@ interface SuccessProps { message: string proceedText: string handleProceed: () => void - isRobotMoving: boolean isExiting: boolean isOnDevice: boolean } export const Success = (props: SuccessProps): JSX.Element => { - const { - message, - proceedText, - handleProceed, - isRobotMoving, - isExiting, - isOnDevice, - } = props + const { message, proceedText, handleProceed, isExiting, isOnDevice } = props const { i18n, t } = useTranslation(['drop_tip_wizard', 'shared']) - return isRobotMoving && !isExiting ? ( - - ) : ( + if (isExiting) { + return + } + + return ( (null) + const hasCleanedUpAndClosed = React.useRef(false) // we should start checking for run deletion only after the maintenance run is created // and the useCurrentRun poll has returned that created id @@ -80,7 +85,6 @@ export function DropTipWizard(props: MaintenanceRunManagerProps): JSX.Element { const { createTargetedMaintenanceRun, - isLoading: isCreateLoading, } = useCreateTargetedMaintenanceRunMutation({ onSuccess: response => { chainRunCommands( @@ -102,6 +106,7 @@ export function DropTipWizard(props: MaintenanceRunManagerProps): JSX.Element { }) .catch(e => e) }, + onError: error => setErrorMessage(error.message), }) const { data: maintenanceRunData } = useCurrentMaintenanceRun({ @@ -140,16 +145,30 @@ export function DropTipWizard(props: MaintenanceRunManagerProps): JSX.Element { }) const handleCleanUpAndClose = (): void => { + if (hasCleanedUpAndClosed.current) return + + hasCleanedUpAndClosed.current = true setIsExiting(true) if (maintenanceRunData?.data.id == null) { closeFlow() } else { - deleteMaintenanceRun(maintenanceRunData?.data.id, { - onSuccess: () => { - closeFlow() - setIsExiting(false) - }, - }) + chainRunCommands( + maintenanceRunData?.data.id, + [ + { + commandType: 'home' as const, + params: { axes: ['leftZ', 'rightZ', 'x', 'y'] }, + }, + ], + true + ) + .then(() => { + deleteMaintenanceRun(maintenanceRunData?.data.id) + }) + .catch(error => { + console.error(error.message) + deleteMaintenanceRun(maintenanceRunData?.data.id) + }) } } @@ -161,7 +180,6 @@ export function DropTipWizard(props: MaintenanceRunManagerProps): JSX.Element { mount={mount} instrumentModelSpecs={instrumentModelSpecs} createMaintenanceRun={createTargetedMaintenanceRun} - isCreateLoading={isCreateLoading} isRobotMoving={isChainCommandMutationLoading || isExiting} handleCleanUpAndClose={handleCleanUpAndClose} chainRunCommands={chainRunCommands} @@ -178,7 +196,6 @@ interface DropTipWizardProps { mount: PipetteData['mount'] createdMaintenanceRunId: string | null createMaintenanceRun: CreateMaintenanceRunType - isCreateLoading: boolean isRobotMoving: boolean isExiting: boolean setErrorMessage: (message: string | null) => void @@ -203,7 +220,6 @@ export const DropTipWizardComponent = ( handleCleanUpAndClose, chainRunCommands, // attachedInstrument, - isCreateLoading, isRobotMoving, createRunCommand, setErrorMessage, @@ -226,8 +242,26 @@ export const DropTipWizardComponent = ( : null const isFinalStep = currentStepIndex === DropTipWizardSteps.length - 1 + React.useEffect(() => { + if (createdMaintenanceRunId == null) { + createMaintenanceRun({}).catch((e: Error) => + setErrorMessage(`Error creating maintenance run: ${e.message}`) + ) + } + }, []) + const goBack = (): void => { - setCurrentStepIndex(isFinalStep ? currentStepIndex : currentStepIndex - 1) + if (createdMaintenanceRunId != null) { + retractAllAxesAndSavePosition() + .then(() => + setCurrentStepIndex( + isFinalStep ? currentStepIndex : currentStepIndex - 1 + ) + ) + .catch((e: Error) => + setErrorMessage(`Error issuing jog command: ${e.message}`) + ) + } } const proceed = (): void => { @@ -238,7 +272,7 @@ export const DropTipWizardComponent = ( } } - const handleJog: Jog = (axis, dir, step) => { + const handleJog: Jog = (axis: Axis, dir: Sign, step: StepSize): void => { if (createdMaintenanceRunId != null) { createRunCommand({ maintenanceRunId: createdMaintenanceRunId, @@ -248,11 +282,9 @@ export const DropTipWizardComponent = ( }, waitUntilComplete: true, timeout: JOG_COMMAND_TIMEOUT_MS, - }) - .then(data => {}) - .catch((e: Error) => - setErrorMessage(`Error issuing jog command: ${e.message}`) - ) + }).catch((e: Error) => + setErrorMessage(`Error issuing jog command: ${e.message}`) + ) } } @@ -269,24 +301,8 @@ export const DropTipWizardComponent = ( ) const commands: CreateCommand[] = [ { - commandType: 'retractAxis' as const, - params: { - axis: 'leftZ', - }, - }, - { - commandType: 'retractAxis' as const, - params: { - axis: 'rightZ', - }, - }, - { - commandType: 'retractAxis' as const, - params: { axis: 'x' }, - }, - { - commandType: 'retractAxis' as const, - params: { axis: 'y' }, + commandType: 'home' as const, + params: { axes: ['leftZ', 'rightZ', 'x', 'y'] }, }, { commandType: 'savePosition' as const, @@ -321,10 +337,9 @@ export const DropTipWizardComponent = ( }) } - const moveToXYCoordinate = ( - x: number, - y: number - ): Promise => { + const moveToAddressableArea = ( + addressableArea: string + ): Promise => { if (createdMaintenanceRunId == null) return Promise.reject( new Error('no maintenance run present to send move commands to') @@ -333,28 +348,18 @@ export const DropTipWizardComponent = ( return retractAllAxesAndSavePosition() .then(currentPosition => { if (currentPosition != null) { - return chainRunCommands( - createdMaintenanceRunId, - [ - { - commandType: 'moveRelative', - params: { - pipetteId: MANAGED_PIPETTE_ID, - distance: y - currentPosition.y, - axis: 'y', - }, - }, - { - commandType: 'moveRelative', - params: { - pipetteId: MANAGED_PIPETTE_ID, - distance: x - currentPosition.x, - axis: 'x', - }, + return createRunCommand({ + maintenanceRunId: createdMaintenanceRunId, + command: { + commandType: 'moveToAddressableArea', + params: { + pipetteId: MANAGED_PIPETTE_ID, + addressableAreaName: addressableArea, + offset: { x: 0, y: 0, z: currentPosition.z - 10 }, }, - ], - true - ) + }, + waitUntilComplete: true, + }) } else return null }) .catch(e => { @@ -368,7 +373,10 @@ export const DropTipWizardComponent = ( modalContent = ( { + hasInitiatedExit.current = true + confirmExit() + }} isRobotMoving={isRobotMoving} /> ) @@ -391,10 +399,9 @@ export const DropTipWizardComponent = ( ) @@ -416,7 +423,10 @@ export const DropTipWizardComponent = ( setShouldDispenseLiquid(null)} + handleGoBack={() => { + setCurrentStepIndex(0) + setShouldDispenseLiquid(null) + }} title={ currentStep === CHOOSE_BLOWOUT_LOCATION ? i18n.format(t('choose_blowout_location'), 'capitalize') @@ -429,7 +439,7 @@ export const DropTipWizardComponent = ( components={{ block: }} /> } - moveToXYCoordinate={moveToXYCoordinate} + moveToAddressableArea={moveToAddressableArea} isRobotMoving={isRobotMoving} isOnDevice={isOnDevice} setErrorMessage={setErrorMessage} @@ -463,13 +473,7 @@ export const DropTipWizardComponent = ( ], true ) - .then(() => { - retractAllAxesAndSavePosition() - .then(() => proceed()) - .catch(e => - setErrorMessage(`Error moving to position: ${e.message}`) - ) - }) + .then(() => proceed()) .catch(e => setErrorMessage( `Error issuing ${ @@ -511,19 +515,16 @@ export const DropTipWizardComponent = ( ? i18n.format(t('shared:continue'), 'capitalize') : i18n.format(t('shared:exit'), 'capitalize') } - isRobotMoving={isRobotMoving} isExiting={isExiting} isOnDevice={isOnDevice} /> ) } - let handleExit: (() => void) | null = confirmExit - if (isRobotMoving || showConfirmExit) { - handleExit = null - } else if (errorMessage != null) { - handleExit = handleCleanUpAndClose - } + const hasInitiatedExit = React.useRef(false) + let handleExit: () => void = () => null + if (!hasInitiatedExit.current) handleExit = confirmExit + else if (errorMessage != null) handleExit = handleCleanUpAndClose const wizardHeader = ( { }) it('renders the drop tip wizard when Drop tips is clicked', () => { - const [{ getByTestId, getByText }] = render(MOCK_PIPETTE) + const [{ getByTestId, getByText, getAllByText }] = render(MOCK_PIPETTE) const btn = getByTestId('testButton') fireEvent.click(btn) fireEvent.click(getByText('Drop tips')) - getByText('Before you begin, do you need to preserve aspirated liquid?') + expect(getAllByText('Drop tips')).toHaveLength(2) }) it('renders the gripper calibration wizard when recalibrate is clicked', () => { From 9a3065fc876bb217b3cd2ce8093d1450894a242b Mon Sep 17 00:00:00 2001 From: Brent Hagen Date: Fri, 15 Dec 2023 12:05:49 -0500 Subject: [PATCH 06/32] fix(app): get addressable area from configured deck for drop tip wizard (#14210) translates a selected "slot" to an addressable area compatible with the configured deck and selected pipette. closes RQA-2099 --- .../DropTipWizard/ChooseLocation.tsx | 4 +- .../getAddressableAreaFromConfig.ts | 87 +++++++++++++++++++ app/src/organisms/DropTipWizard/index.tsx | 29 ++++++- 3 files changed, 114 insertions(+), 6 deletions(-) create mode 100644 app/src/organisms/DropTipWizard/getAddressableAreaFromConfig.ts diff --git a/app/src/organisms/DropTipWizard/ChooseLocation.tsx b/app/src/organisms/DropTipWizard/ChooseLocation.tsx index 5a384b217ef..d4919f803d4 100644 --- a/app/src/organisms/DropTipWizard/ChooseLocation.tsx +++ b/app/src/organisms/DropTipWizard/ChooseLocation.tsx @@ -27,7 +27,7 @@ import { InProgressModal } from '../../molecules/InProgressModal/InProgressModal import { TwoUpTileLayout } from '../LabwarePositionCheck/TwoUpTileLayout' import type { CommandData } from '@opentrons/api-client' -import type { RobotType } from '@opentrons/shared-data' +import type { AddressableAreaName, RobotType } from '@opentrons/shared-data' // TODO: get help link article URL // const NEED_HELP_URL = '' @@ -39,7 +39,7 @@ interface ChooseLocationProps { body: string | JSX.Element robotType: RobotType moveToAddressableArea: ( - addressableArea: string + addressableArea: AddressableAreaName ) => Promise isRobotMoving: boolean isOnDevice: boolean diff --git a/app/src/organisms/DropTipWizard/getAddressableAreaFromConfig.ts b/app/src/organisms/DropTipWizard/getAddressableAreaFromConfig.ts new file mode 100644 index 00000000000..021b84ccc09 --- /dev/null +++ b/app/src/organisms/DropTipWizard/getAddressableAreaFromConfig.ts @@ -0,0 +1,87 @@ +import { + EIGHT_CHANNEL_WASTE_CHUTE_ADDRESSABLE_AREA, + getCutoutIdForAddressableArea, + getDeckDefFromRobotType, + MOVABLE_TRASH_ADDRESSABLE_AREAS, + NINETY_SIX_CHANNEL_WASTE_CHUTE_ADDRESSABLE_AREA, + ONE_CHANNEL_WASTE_CHUTE_ADDRESSABLE_AREA, + WASTE_CHUTE_ADDRESSABLE_AREAS, +} from '@opentrons/shared-data' + +import type { + AddressableAreaName, + DeckConfiguration, + RobotType, +} from '@opentrons/shared-data' + +export function getAddressableAreaFromConfig( + addressableArea: AddressableAreaName, + deckConfig: DeckConfiguration, + pipetteChannels: number, + robotType: RobotType +): AddressableAreaName | null { + const deckDef = getDeckDefFromRobotType(robotType) + + let addressableAreaFromConfig: AddressableAreaName | null = null + + // match the cutout id to aa + const cutoutIdForAddressableArea = getCutoutIdForAddressableArea( + addressableArea, + deckDef.cutoutFixtures + ) + + // get addressable areas provided by current deck config + const configuredCutoutFixture = + deckConfig.find(fixture => fixture.cutoutId === cutoutIdForAddressableArea) + ?.cutoutFixtureId ?? null + + const providedAddressableAreas = + cutoutIdForAddressableArea != null + ? deckDef.cutoutFixtures.find( + fixture => fixture.id === configuredCutoutFixture + )?.providesAddressableAreas[cutoutIdForAddressableArea] ?? [] + : [] + + // check if configured cutout fixture id provides target addressableArea + if (providedAddressableAreas.includes(addressableArea)) { + addressableAreaFromConfig = addressableArea + } else if ( + // if no, check if provides a movable trash + providedAddressableAreas.some(aa => + MOVABLE_TRASH_ADDRESSABLE_AREAS.includes(aa) + ) + ) { + addressableAreaFromConfig = providedAddressableAreas[0] + } else if ( + // if no, check if provides waste chute + providedAddressableAreas.some(aa => + WASTE_CHUTE_ADDRESSABLE_AREAS.includes(aa) + ) + ) { + // match number of channels to provided waste chute addressable area + if ( + pipetteChannels === 1 && + providedAddressableAreas.includes( + ONE_CHANNEL_WASTE_CHUTE_ADDRESSABLE_AREA + ) + ) { + addressableAreaFromConfig = ONE_CHANNEL_WASTE_CHUTE_ADDRESSABLE_AREA + } else if ( + pipetteChannels === 8 && + providedAddressableAreas.includes( + EIGHT_CHANNEL_WASTE_CHUTE_ADDRESSABLE_AREA + ) + ) { + addressableAreaFromConfig = EIGHT_CHANNEL_WASTE_CHUTE_ADDRESSABLE_AREA + } else if ( + pipetteChannels === 96 && + providedAddressableAreas.includes( + NINETY_SIX_CHANNEL_WASTE_CHUTE_ADDRESSABLE_AREA + ) + ) { + addressableAreaFromConfig = NINETY_SIX_CHANNEL_WASTE_CHUTE_ADDRESSABLE_AREA + } + } + + return addressableAreaFromConfig +} diff --git a/app/src/organisms/DropTipWizard/index.tsx b/app/src/organisms/DropTipWizard/index.tsx index e359aeaab1d..53ec902a37b 100644 --- a/app/src/organisms/DropTipWizard/index.tsx +++ b/app/src/organisms/DropTipWizard/index.tsx @@ -14,6 +14,7 @@ import { useCreateMaintenanceCommandMutation, useDeleteMaintenanceRunMutation, useCurrentMaintenanceRun, + useDeckConfigurationQuery, CreateMaintenanceRunType, } from '@opentrons/react-api-client' @@ -29,6 +30,7 @@ import { import { StyledText } from '../../atoms/text' import { Jog } from '../../molecules/JogControls' import { ExitConfirmation } from './ExitConfirmation' +import { getAddressableAreaFromConfig } from './getAddressableAreaFromConfig' import { getDropTipWizardSteps } from './getDropTipWizardSteps' import { BLOWOUT_SUCCESS, @@ -50,6 +52,8 @@ import type { RobotType, SavePositionRunTimeCommand, CreateCommand, + DeckConfiguration, + AddressableAreaName, } from '@opentrons/shared-data' import type { Axis, Sign, StepSize } from '../../molecules/JogControls/types' @@ -71,6 +75,8 @@ export function DropTipWizard(props: MaintenanceRunManagerProps): JSX.Element { } = useChainMaintenanceCommands() const { createMaintenanceCommand } = useCreateMaintenanceCommandMutation() + const deckConfig = useDeckConfigurationQuery().data ?? [] + const [createdMaintenanceRunId, setCreatedMaintenanceRunId] = React.useState< string | null >(null) @@ -187,6 +193,7 @@ export function DropTipWizard(props: MaintenanceRunManagerProps): JSX.Element { errorMessage={errorMessage} setErrorMessage={setErrorMessage} isExiting={isExiting} + deckConfig={deckConfig} /> ) } @@ -208,6 +215,7 @@ interface DropTipWizardProps { typeof useCreateMaintenanceCommandMutation >['createMaintenanceCommand'] instrumentModelSpecs: PipetteModelSpecs + deckConfig: DeckConfiguration maintenanceRunId?: string } @@ -227,6 +235,7 @@ export const DropTipWizardComponent = ( isExiting, createdMaintenanceRunId, instrumentModelSpecs, + deckConfig, } = props const isOnDevice = useSelector(getIsOnDevice) const { t, i18n } = useTranslation('drop_tip_wizard') @@ -338,7 +347,7 @@ export const DropTipWizardComponent = ( } const moveToAddressableArea = ( - addressableArea: string + addressableArea: AddressableAreaName ): Promise => { if (createdMaintenanceRunId == null) return Promise.reject( @@ -347,15 +356,27 @@ export const DropTipWizardComponent = ( return retractAllAxesAndSavePosition() .then(currentPosition => { - if (currentPosition != null) { + const addressableAreaFromConfig = getAddressableAreaFromConfig( + addressableArea, + deckConfig, + instrumentModelSpecs.channels, + robotType + ) + + const zOffset = + addressableAreaFromConfig === addressableArea + ? (currentPosition as Coordinates).z - 10 + : 0 + + if (currentPosition != null && addressableAreaFromConfig != null) { return createRunCommand({ maintenanceRunId: createdMaintenanceRunId, command: { commandType: 'moveToAddressableArea', params: { pipetteId: MANAGED_PIPETTE_ID, - addressableAreaName: addressableArea, - offset: { x: 0, y: 0, z: currentPosition.z - 10 }, + addressableAreaName: addressableAreaFromConfig, + offset: { x: 0, y: 0, z: zOffset }, }, }, waitUntilComplete: true, From 8c4f1d38cb63654c5dec7ba69fd6eb445c53f920 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Fri, 15 Dec 2023 12:56:10 -0500 Subject: [PATCH 07/32] fix(app-shell): use a wrapping stream for usb (#14214) Implement the tcp socket emulator by creating a separate stream that is piped through to the usb connection via its downward interface rather than using the same stream for both the port and the socket or pipelining the two streams together. This implementation allows the lifecycles of the USB port connection, which we want to be the same as the physical robot connection; and the tcp socket emulators, which we want to be more like tcp sockets; to be separate, which really increases reliability because we don't have the port going up and down all the time anymore. This reduces spurious disconnects. It will also allow us to add socket activity timeouts to help with the windows dropping data problem, though we can't really do that until the http requests that cause synchronous actions that we make get keep alive streaming responses. --- app-shell/src/usb.ts | 124 +++++------ usb-bridge/node-client/src/usb-agent.ts | 277 ++++++++++++++---------- 2 files changed, 225 insertions(+), 176 deletions(-) diff --git a/app-shell/src/usb.ts b/app-shell/src/usb.ts index 81d1afdade1..ee402093770 100644 --- a/app-shell/src/usb.ts +++ b/app-shell/src/usb.ts @@ -34,25 +34,50 @@ let usbFetchInterval: NodeJS.Timeout export function getSerialPortHttpAgent(): SerialPortHttpAgent | undefined { return usbHttpAgent } -export function createSerialPortHttpAgent(path: string): void { - const serialPortHttpAgent = new SerialPortHttpAgent({ - maxFreeSockets: 1, - maxSockets: 1, - maxTotalSockets: 1, - keepAlive: true, - keepAliveMsecs: Infinity, - path, - logger: usbLog, - timeout: 100000, - }) - usbHttpAgent = serialPortHttpAgent +export function createSerialPortHttpAgent( + path: string, + onComplete: (err: Error | null, agent?: SerialPortHttpAgent) => void +): void { + if (usbHttpAgent != null) { + onComplete( + new Error('Tried to make a USB http agent when one already existed') + ) + } else { + usbHttpAgent = new SerialPortHttpAgent( + { + maxFreeSockets: 1, + maxSockets: 1, + maxTotalSockets: 1, + keepAlive: true, + keepAliveMsecs: Infinity, + path, + logger: usbLog, + timeout: 100000, + }, + (err, agent?) => { + if (err != null) { + usbHttpAgent = undefined + } + onComplete(err, agent) + } + ) + } } -export function destroyUsbHttpAgent(): void { +export function destroyAndStopUsbHttpRequests(dispatch: Dispatch): void { if (usbHttpAgent != null) { usbHttpAgent.destroy() } usbHttpAgent = undefined + ipcMain.removeHandler('usb:request') + dispatch(usbRequestsStop()) + // handle any additional invocations of usb:request + ipcMain.handle('usb:request', () => + Promise.resolve({ + status: 400, + statusText: 'USB robot disconnected', + }) + ) } function isUsbDeviceOt3(device: UsbDevice): boolean { @@ -115,42 +140,11 @@ function pollSerialPortAndCreateAgent(dispatch: Dispatch): void { } usbFetchInterval = setInterval(() => { // already connected to an Opentrons robot via USB - if (getSerialPortHttpAgent() != null) { - return - } - usbLog.debug('fetching serialport list') - fetchSerialPortList() - .then((list: PortInfo[]) => { - const ot3UsbSerialPort = list.find( - port => - port.productId?.localeCompare(DEFAULT_PRODUCT_ID, 'en-US', { - sensitivity: 'base', - }) === 0 && - port.vendorId?.localeCompare(DEFAULT_VENDOR_ID, 'en-US', { - sensitivity: 'base', - }) === 0 - ) - - if (ot3UsbSerialPort == null) { - usbLog.debug('no OT-3 serial port found') - return - } - - createSerialPortHttpAgent(ot3UsbSerialPort.path) - // remove any existing handler - ipcMain.removeHandler('usb:request') - ipcMain.handle('usb:request', usbListener) - - dispatch(usbRequestsStart()) - }) - .catch(e => - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - usbLog.debug(`fetchSerialPortList error ${e?.message ?? 'unknown'}`) - ) + tryCreateAndStartUsbHttpRequests(dispatch) }, 10000) } -function startUsbHttpRequests(dispatch: Dispatch): void { +function tryCreateAndStartUsbHttpRequests(dispatch: Dispatch): void { fetchSerialPortList() .then((list: PortInfo[]) => { const ot3UsbSerialPort = list.find( @@ -165,17 +159,22 @@ function startUsbHttpRequests(dispatch: Dispatch): void { // retry if no OT-3 serial port found - usb-detection and serialport packages have race condition if (ot3UsbSerialPort == null) { - usbLog.debug('no OT-3 serial port found, retrying') - setTimeout(() => startUsbHttpRequests(dispatch), 1000) + usbLog.debug('no OT-3 serial port found') return } - - createSerialPortHttpAgent(ot3UsbSerialPort.path) - // remove any existing handler - ipcMain.removeHandler('usb:request') - ipcMain.handle('usb:request', usbListener) - - dispatch(usbRequestsStart()) + if (usbHttpAgent == null) { + createSerialPortHttpAgent(ot3UsbSerialPort.path, (err, agent?) => { + if (err != null) { + const message = err?.message ?? err + usbLog.error(`Failed to create serial port: ${message}`) + } + if (agent) { + ipcMain.removeHandler('usb:request') + ipcMain.handle('usb:request', usbListener) + dispatch(usbRequestsStart()) + } + }) + } }) .catch(e => // eslint-disable-next-line @typescript-eslint/restrict-template-expressions @@ -188,27 +187,18 @@ export function registerUsb(dispatch: Dispatch): (action: Action) => unknown { switch (action.type) { case SYSTEM_INFO_INITIALIZED: if (action.payload.usbDevices.find(isUsbDeviceOt3) != null) { - startUsbHttpRequests(dispatch) + tryCreateAndStartUsbHttpRequests(dispatch) } pollSerialPortAndCreateAgent(dispatch) break case USB_DEVICE_ADDED: if (isUsbDeviceOt3(action.payload.usbDevice)) { - startUsbHttpRequests(dispatch) + tryCreateAndStartUsbHttpRequests(dispatch) } break case USB_DEVICE_REMOVED: if (isUsbDeviceOt3(action.payload.usbDevice)) { - destroyUsbHttpAgent() - ipcMain.removeHandler('usb:request') - dispatch(usbRequestsStop()) - // handle any additional invocations of usb:request - ipcMain.handle('usb:request', () => - Promise.resolve({ - status: 400, - statusText: 'USB robot disconnected', - }) - ) + destroyAndStopUsbHttpRequests(dispatch) } break } diff --git a/usb-bridge/node-client/src/usb-agent.ts b/usb-bridge/node-client/src/usb-agent.ts index 62639f23796..b4a2bf933e2 100644 --- a/usb-bridge/node-client/src/usb-agent.ts +++ b/usb-bridge/node-client/src/usb-agent.ts @@ -1,6 +1,6 @@ import * as http from 'http' import agent from 'agent-base' -import type { Duplex } from 'stream' +import { Duplex } from 'stream' import { SerialPort } from 'serialport' @@ -110,38 +110,95 @@ export function createSerialPortListMonitor( return { start, stop } } -class SerialPortSocket extends SerialPort { - // added these to squash keepAlive errors - setKeepAlive(): void {} +interface SerialPortHttpAgentOptions extends AgentOptions { + path: string + logger: Logger +} - unref(): SerialPortSocket { - return this +function socketEmulatorFromPort(port: SerialPort): Socket { + // build a duplex stream to act as a socket that we can give to node https internals, linked + // to an open usb serial port. + // + // this is a separate stream rather than just passing in the port so that we can sever the + // lifetimes and lifecycles of the socket and the port. sockets want to be closed and opened all + // the time by node http internals, and we don't want that for the port since opening and closing it + // can take a while. this lets us open and close and create and destroy sockets at will while not + // affecting the port. + + // unfortunately, because we need to sever the lifecycles, we can't use node stream pipelining + // since half the point of node stream pipelining is to link stream lifecycles. instead, we do a + // custom duplex implementation whose lower interface talks to the upper interface of the port... + // which is something that's really annoying without using pipelining, which we can't use. so + // this closed-over mutable doRead has to stand in for the pause event propagating down; we have to + // add or remove data listeners to the port stream to propagate read backpressure. + let doRead = false + const socket = new Duplex({ + write(chunk, encoding, cb) { + return port.write(chunk, encoding, cb) + }, + read() { + if (!doRead) { + port.on('data', dataForwarder) + doRead = true + } + }, + }) as Socket + + const dataForwarder = (chunk: any): void => { + if (doRead) { + doRead = socket.push(chunk) + if (!doRead) { + port.removeListener('data', dataForwarder) + } + } } - setTimeout(): void {} - - ref(): SerialPortSocket { - return this + // since this socket is independent from the port, we can do stuff like "have an activity timeout" + // without worrying that it will kill the socket + let currentTimeout: NodeJS.Timeout | null = null + const refreshTimeout = (): void => { + currentTimeout?.refresh() } - - // We never actually really want to destroy our serial port sockets, but - // the abort logic (at least) in node http client actually has a call stack - // that requires the socket close event to happen (???) so this is for that. - // We only really seem to abort when there's a 3xx return because we use - // npm follow-redirects and that aborts on a 3xx - destroy(): void { - if (!!!this.destroyed) { - this.destroyed = true - this.close() + socket.on('data', refreshTimeout) + socket.setTimeout = (timeout, callable?) => { + currentTimeout !== null && clearTimeout(currentTimeout) + if (timeout === 0 && currentTimeout !== null) { + currentTimeout = null + } else if (timeout !== 0) { + currentTimeout = setTimeout(() => { + console.log('socket timed out') + socket.emit('timeout') + }, timeout) + if (callable != null) { + socket.once('timeout', callable) + } } + + return socket } + // important: without this we'll leak sockets since the port event emitter will hold a ref to dataForwarder which + // closes over the socket + socket.on('close', () => { + port.removeListener('data', dataForwarder) + }) - _httpMessage: { shouldKeepAlive: boolean } | undefined = undefined -} + // some little functions to have the right shape for the http internals + socket.ref = () => socket + socket.unref = () => socket + socket.setKeepAlive = () => { + return socket + } + socket.setNoDelay = () => { + return socket + } -interface SerialPortHttpAgentOptions extends AgentOptions { - path: string - logger: Logger + socket.on('finish', () => { + socket.emit('close') + }) + socket.on('close', () => { + currentTimeout && clearTimeout(currentTimeout) + }) + return socket } const kOnKeylog = Symbol.for('onkeylog') @@ -151,24 +208,75 @@ class SerialPortHttpAgent extends http.Agent { declare sockets: NodeJS.Dict declare emit: ( event: string, - socket: SerialPortSocket, + socket: Socket, options: NodeJS.Dict ) => void declare getName: (options: NodeJS.Dict) => string - declare removeSocket: ( - socket: SerialPortSocket, - options: NodeJS.Dict - ) => void; + declare removeSocket: (socket: Socket, options: NodeJS.Dict) => void; // node can assign a keylogger to the agent for debugging, this allows adding the keylog listener to the event declare [kOnKeylog]: (...args: unknown[]) => void - constructor(options: SerialPortHttpAgentOptions) { + constructor( + options: SerialPortHttpAgentOptions, + onComplete: (err: Error | null, agent?: SerialPortHttpAgent) => void + ) { super(options) this.options = options + const openRetryer: (err: Error | null) => void = err => { + if (err != null) { + if (this.remainingRetries > 0 && !this.destroyed) { + const message = err?.message ?? err + this.log( + 'info', + `Failed to open port: ${message} , retrying ${this.remainingRetries} more times` + ) + this.remainingRetries-- + setTimeout( + () => this.port.open(openRetryer), + SOCKET_OPEN_RETRY_TIME_MS + ) + } else if (!this.destroyed) { + const message = err?.message ?? err + this.log( + 'info', + `Failed to open port after ${this.remainingRetries} attempts: ${message}` + ) + this.destroy() + onComplete(err) + } else { + this.log( + 'info', + `Cancelling open attempts because the agent was destroyed` + ) + onComplete(new Error('Agent destroyed while opening')) + } + } else if (!this.destroyed) { + this.log('info', `Port ${this.options.path} now open`) + onComplete(null, this) + } else { + this.log('info', `Port was opened but agent is now destroyed, closing`) + if (this.port.isOpen) { + this.port.close() + } + onComplete(new Error('Agent destroyed while opening')) + } + } + this.log( + 'info', + `creating and opening serial port for ${this.options.path}` + ) + this.port = new SerialPort( + { path: this.options.path, baudRate: 1152000, autoOpen: true }, + openRetryer + ) } + port: SerialPort + remainingRetries: number = MAX_SOCKET_CREATE_RETRIES + destroyed: boolean = false + // TODO: add method to close port (or destroy agent) options: { @@ -185,77 +293,49 @@ class SerialPortHttpAgent extends http.Agent { this.options.logger[level](msg, meta) } + destroy(): void { + this.destroyed = true + this.port.destroy(new Error('Agent was destroyed')) + } + createSocket( req: http.ClientRequest, options: NodeJS.Dict, - cb: Function + cb: (err: Error | string | null, stream?: Duplex) => void ): void { // copied from _http_agent.js, replacing this.createConnection - this.log('info', `creating usb socket at ${this.options.path}`) + this.log('info', `creating usb socket wrapper to ${this.options.path}`) options = { __proto__: null, ...options, ...this.options } const name = this.getName(options) options._agentKey = name options.encoding = null - // We preemptively increase the socket count and then reduce it if we - // actually failed because more requests will come in as soon as this function - // function finishes and if we don't increment it here those messages will also - // try and make new sockets - this.totalSocketCount++ - const oncreate = (err: any | null, s?: SerialPortSocket): void => { - if (err != null) { - this.totalSocketCount-- - return cb(err) - } - if (this.sockets[name] == null) { - this.sockets[name] = [] - } - this.sockets[name]?.push((s as unknown) as Socket) - this.log( - 'debug', - `sockets ${name} ${this.sockets[name]?.length ?? ''} ${ - this.totalSocketCount - }` - ) - installListeners(this, s as SerialPortSocket, options) - cb(null, s) + if (this.totalSocketCount >= 1) { + this.log('error', `tried to create more than one socket wrapper`) + cb(new Error('Cannot create more than one USB port wrapper')) + return } - // we do retries via recursion because this is all callback based anyway - const createSocketInner: ( - req: http.ClientRequest, - options: NodeJS.Dict, - cb: Function, - remainingRetries: number - ) => void = (req, options, cb, remainingRetries) => { - const socket: SerialPortSocket = new SerialPortSocket({ - path: this.options.path, - baudRate: 1152000, - // setting autoOpen false makes the rest of the logic a little easier because - // we always go through the "open-after-constructor" codepath - autoOpen: false, - }) - socket.open(err => { - if (err) { - if (remainingRetries > 0) { - setTimeout( - () => createSocketInner(req, options, cb, remainingRetries - 1), - SOCKET_OPEN_RETRY_TIME_MS - ) - } else { - oncreate(err) - } - } else { - oncreate(err, socket) - } - }) + if (!this.port.isOpen) { + this.log('error', `tried to create usb socket wrapper with closed port`) + cb(new Error('Underlying USB port is closed')) + return } - createSocketInner(req, options, cb, MAX_SOCKET_CREATE_RETRIES) + + const wrapper = socketEmulatorFromPort(this.port) + this.totalSocketCount++ + installListeners(this, wrapper, options) + this.log('info', `created usb socket wrapper writable: ${wrapper.writable}`) + cb(null, wrapper) + setImmediate(() => { + wrapper.emit('connect') + wrapper.emit('ready') + }) } } // most copied from _http_agent.js; onData and onFinish listeners added to log and close serial port function installListeners( agent: SerialPortHttpAgent, - s: SerialPortSocket, + s: Socket, options: { [k: string]: unknown } ): void { const onFree: () => void = () => { @@ -267,19 +347,10 @@ function installListeners( // the function, but we need the entire thing except like one conditional so we do this. agent.log('debug', 'CLIENT socket onFree') - // need to emit free to attach listeners to serialport - if (s._httpMessage) { - s._httpMessage.shouldKeepAlive = true - } agent.emit('free', s, options) } s.on('free', onFree) - s.on('open', () => { - s.emit('connect') - s.emit('ready') - }) - function onError(err: Error): void { agent.log('error', `CLIENT socket onError: ${err?.message}`) } @@ -287,25 +358,13 @@ function installListeners( function onClose(): void { agent.log('debug', 'CLIENT socket onClose') - // the 'close' event is emitted both by the serial port stream when it closes - // the serial port (yay) and by both the readable and writable streams that the - // serial port inherits from when they close which has nothing to do with the serial - // port (boo!) so if we get a close event we need to check if we're actually closed - // and if we're not do a real close (and also only remove the socket from the agent - // if it's real) - - if (s.isOpen) { - s.close() - } else { - agent.totalSocketCount-- - agent.removeSocket(s, options) - } + agent.totalSocketCount-- + agent.removeSocket(s, options) } s.on('close', onClose) function onFinish(): void { - agent.log('info', 'socket finishing: closing serialport') - s.close() + agent.log('info', 'socket finishing') } s.on('finish', onFinish) From 3f1c70ff8b5fd4fa31538c47cfb089c721f77fe3 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Fri, 15 Dec 2023 13:00:21 -0500 Subject: [PATCH 08/32] fix(api): do not tip check during attach (#14216) Since on the 96, that moves motors while someone is holding it in their hands. --- api/src/opentrons/hardware_control/ot3api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/opentrons/hardware_control/ot3api.py b/api/src/opentrons/hardware_control/ot3api.py index 5d429d7e11f..45e134c3342 100644 --- a/api/src/opentrons/hardware_control/ot3api.py +++ b/api/src/opentrons/hardware_control/ot3api.py @@ -737,7 +737,7 @@ async def _configure_instruments(self) -> None: """Configure instruments""" await self.set_gantry_load(self._gantry_load_from_instruments()) await self.refresh_positions() - await self.reset_tip_detectors() + await self.reset_tip_detectors(False) async def reset_tip_detectors( self, From 9d4ce722c5fa1e2007b458b49c434413732af336 Mon Sep 17 00:00:00 2001 From: Jamey H Date: Fri, 15 Dec 2023 13:06:05 -0500 Subject: [PATCH 09/32] refactor(app): Remove drop tip banner after OT-2 protocol run (#14212) Closes RQA-2106 * refactor(app): remove drop tip banner for ot2 after protocol runs * refactor(app): add more error handling to drop tip wizard moveToAddressableArea * style(app): add sensible text when drop tip launches * fix(app): cancelling a run should not unrender drop tip banner * fix(app): include fixed trash as a moveToAddressableArea zoffset exception * fix(app): do not show drop tip/blowout confirmation until moveToAddressableArea has completed * refactor(app): add even more error handling - this time to the droptip and blowout commands --- .../localization/en/drop_tip_wizard.json | 1 + .../Devices/ProtocolRun/ProtocolRunHeader.tsx | 57 ++++++++++--------- .../__tests__/ProtocolRunHeader.test.tsx | 4 +- .../DropTipWizard/BeforeBeginning.tsx | 10 +++- app/src/organisms/DropTipWizard/index.tsx | 46 ++++++++++----- 5 files changed, 73 insertions(+), 45 deletions(-) diff --git a/app/src/assets/localization/en/drop_tip_wizard.json b/app/src/assets/localization/en/drop_tip_wizard.json index e8fb48db0bf..66924d00210 100644 --- a/app/src/assets/localization/en/drop_tip_wizard.json +++ b/app/src/assets/localization/en/drop_tip_wizard.json @@ -12,6 +12,7 @@ "drop_tips": "drop tips", "error_dropping_tips": "Error dropping tips", "exit_screen_title": "Exit before completing drop tip?", + "getting_ready": "Getting ready…", "go_back": "go back", "move_to_slot": "move to slot", "no_proceed_to_drop_tip": "No, proceed to tip removal", diff --git a/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx b/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx index 98002f6d3a5..8c38f917afa 100644 --- a/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx +++ b/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx @@ -203,33 +203,35 @@ export function ProtocolRunHeader({ } React.useEffect(() => { - // Reset drop tip state when a new run occurs. - if (runStatus === RUN_STATUS_IDLE) { - setShowDropTipBanner(true) - setPipettesWithTip([]) - } else if (runStatus != null && RUN_OVER_STATUSES.includes(runStatus)) { - getPipettesWithTipAttached({ - host, - runId, - runRecord, - attachedInstruments, - isFlex, - }) - .then(pipettesWithTipAttached => { - const newPipettesWithTipAttached = pipettesWithTipAttached.map( - pipette => { - const specs = getPipetteModelSpecs(pipette.instrumentModel) - return { - specs, - mount: pipette.mount, - } - } - ) - setPipettesWithTip(() => newPipettesWithTipAttached) - }) - .catch(e => { - console.log(`Error checking pipette tip attachement state: ${e}`) + if (isFlex) { + // Reset drop tip state when a new run occurs. + if (runStatus === RUN_STATUS_IDLE) { + setShowDropTipBanner(true) + setPipettesWithTip([]) + } else if (runStatus != null && RUN_OVER_STATUSES.includes(runStatus)) { + getPipettesWithTipAttached({ + host, + runId, + runRecord, + attachedInstruments, + isFlex, }) + .then(pipettesWithTipAttached => { + const newPipettesWithTipAttached = pipettesWithTipAttached.map( + pipette => { + const specs = getPipetteModelSpecs(pipette.instrumentModel) + return { + specs, + mount: pipette.mount, + } + } + ) + setPipettesWithTip(() => newPipettesWithTipAttached) + }) + .catch(e => { + console.log(`Error checking pipette tip attachement state: ${e}`) + }) + } } }, [runStatus, attachedInstruments, host, runId, runRecord, isFlex]) @@ -247,9 +249,8 @@ export function ProtocolRunHeader({ ...robotAnalyticsData, }, }) - closeCurrentRun() } - }, [runStatus, isRunCurrent, runId, closeCurrentRun]) + }, [runStatus, isRunCurrent, runId]) const startedAtTimestamp = startedAt != null ? formatTimestamp(startedAt) : EMPTY_TIMESTAMP diff --git a/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx b/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx index ac33aaa677d..aa65ae218e0 100644 --- a/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx +++ b/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx @@ -523,7 +523,7 @@ describe('ProtocolRunHeader', () => { data: { data: { ...mockIdleUnstartedRun, current: true } }, } as UseQueryResult) render() - expect(mockCloseCurrentRun).toBeCalled() + expect(mockCloseCurrentRun).not.toBeCalled() expect(mockTrackProtocolRunEvent).toBeCalled() expect(mockTrackProtocolRunEvent).toBeCalledWith({ name: ANALYTICS_PROTOCOL_RUN_FINISH, @@ -1007,7 +1007,7 @@ describe('ProtocolRunHeader', () => { ).not.toBeInTheDocument() }) - it('renders the drop tip banner when the run is over and a pipette has a tip attached', async () => { + it('renders the drop tip banner when the run is over and a pipette has a tip attached and is a flex', async () => { when(mockUseRunQuery) .calledWith(RUN_ID) .mockReturnValue({ diff --git a/app/src/organisms/DropTipWizard/BeforeBeginning.tsx b/app/src/organisms/DropTipWizard/BeforeBeginning.tsx index 0333827ab54..b5e2a6b00ca 100644 --- a/app/src/organisms/DropTipWizard/BeforeBeginning.tsx +++ b/app/src/organisms/DropTipWizard/BeforeBeginning.tsx @@ -58,7 +58,15 @@ export const BeforeBeginning = ( } if (isRobotMoving || createdMaintenanceRunId == null) { - return + return ( + + ) } if (isOnDevice) { diff --git a/app/src/organisms/DropTipWizard/index.tsx b/app/src/organisms/DropTipWizard/index.tsx index 53ec902a37b..80742606d04 100644 --- a/app/src/organisms/DropTipWizard/index.tsx +++ b/app/src/organisms/DropTipWizard/index.tsx @@ -349,10 +349,11 @@ export const DropTipWizardComponent = ( const moveToAddressableArea = ( addressableArea: AddressableAreaName ): Promise => { - if (createdMaintenanceRunId == null) + if (createdMaintenanceRunId == null) { return Promise.reject( new Error('no maintenance run present to send move commands to') ) + } return retractAllAxesAndSavePosition() .then(currentPosition => { @@ -364,24 +365,36 @@ export const DropTipWizardComponent = ( ) const zOffset = - addressableAreaFromConfig === addressableArea + addressableAreaFromConfig === addressableArea && + addressableAreaFromConfig !== 'fixedTrash' ? (currentPosition as Coordinates).z - 10 : 0 if (currentPosition != null && addressableAreaFromConfig != null) { - return createRunCommand({ - maintenanceRunId: createdMaintenanceRunId, - command: { - commandType: 'moveToAddressableArea', - params: { - pipetteId: MANAGED_PIPETTE_ID, - addressableAreaName: addressableAreaFromConfig, - offset: { x: 0, y: 0, z: zOffset }, + return chainRunCommands( + createdMaintenanceRunId, + [ + { + commandType: 'moveToAddressableArea', + params: { + pipetteId: MANAGED_PIPETTE_ID, + addressableAreaName: addressableAreaFromConfig, + offset: { x: 0, y: 0, z: zOffset }, + }, }, - }, - waitUntilComplete: true, + ], + true + ).then(commandData => { + const error = commandData[0].data.error + if (error != null) { + setErrorMessage(`error moving to position: ${error.detail}`) + } + return null }) - } else return null + } else { + setErrorMessage(`error moving to position: invalid addressable area.`) + return null + } }) .catch(e => { setErrorMessage(`error moving to position: ${e.message}`) @@ -494,7 +507,12 @@ export const DropTipWizardComponent = ( ], true ) - .then(() => proceed()) + .then(commandData => { + const error = commandData[0].data.error + if (error != null) { + setErrorMessage(`error moving to position: ${error.detail}`) + } else proceed() + }) .catch(e => setErrorMessage( `Error issuing ${ From 98422074546a2e99a38d58cfd3a39946e82e0ec2 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Fri, 15 Dec 2023 13:10:25 -0500 Subject: [PATCH 10/32] fix(protocol-engine): Fix hanging after cancellation (#14215) --- .../execution/hardware_stopper.py | 57 +++++++------------ 1 file changed, 20 insertions(+), 37 deletions(-) diff --git a/api/src/opentrons/protocol_engine/execution/hardware_stopper.py b/api/src/opentrons/protocol_engine/execution/hardware_stopper.py index 2b45431fb57..044b1f83daf 100644 --- a/api/src/opentrons/protocol_engine/execution/hardware_stopper.py +++ b/api/src/opentrons/protocol_engine/execution/hardware_stopper.py @@ -50,25 +50,29 @@ def __init__( state_view=state_store, ) + async def _home_everything_except_plungers(self) -> None: + # TODO: Update this once gripper MotorAxis is available in engine. + try: + ot3api = ensure_ot3_hardware(hardware_api=self._hardware_api) + if ( + not self._state_store.config.use_virtual_gripper + and ot3api.has_gripper() + ): + await ot3api.home_z(mount=OT3Mount.GRIPPER) + except HardwareNotSupportedError: + pass + await self._movement_handler.home( + axes=[MotorAxis.X, MotorAxis.Y, MotorAxis.LEFT_Z, MotorAxis.RIGHT_Z] + ) + async def _drop_tip(self) -> None: """Drop currently attached tip, if any, into trash after a run cancel.""" attached_tips = self._state_store.pipettes.get_all_attached_tips() if attached_tips: await self._hardware_api.stop(home_after=False) - # TODO: Update this once gripper MotorAxis is available in engine. - try: - ot3api = ensure_ot3_hardware(hardware_api=self._hardware_api) - if ( - not self._state_store.config.use_virtual_gripper - and ot3api.has_gripper() - ): - await ot3api.home_z(mount=OT3Mount.GRIPPER) - except HardwareNotSupportedError: - pass - await self._movement_handler.home( - axes=[MotorAxis.X, MotorAxis.Y, MotorAxis.LEFT_Z, MotorAxis.RIGHT_Z] - ) + + await self._home_everything_except_plungers() # OT-2 Will only ever use the Fixed Trash Addressable Area if self._state_store.config.robot_type == "OT-2 Standard": @@ -125,28 +129,7 @@ async def do_stop_and_recover( if drop_tips_after_run: await self._drop_tip() await self._hardware_api.stop(home_after=home_after_stop) - - elif home_after_stop: - if len(self._state_store.pipettes.get_all_attached_tips()) == 0: - await self._hardware_api.stop(home_after=home_after_stop) - else: - try: - ot3api = ensure_ot3_hardware(hardware_api=self._hardware_api) - if ( - not self._state_store.config.use_virtual_gripper - and ot3api.has_gripper() - ): - await ot3api.home_z(mount=OT3Mount.GRIPPER) - except HardwareNotSupportedError: - pass - - await self._movement_handler.home( - axes=[ - MotorAxis.X, - MotorAxis.Y, - MotorAxis.LEFT_Z, - MotorAxis.RIGHT_Z, - ] - ) else: - await self._hardware_api.stop(home_after=home_after_stop) + await self._hardware_api.stop(home_after=False) + if home_after_stop: + await self._home_everything_except_plungers() From ef937eef69516e63a1e003674519420ef3efc14e Mon Sep 17 00:00:00 2001 From: koji Date: Fri, 15 Dec 2023 14:06:05 -0500 Subject: [PATCH 11/32] fix(app): e-stop modal button state issue (#14217) * fix(app): fix estop modal button state issue --- .../EmergencyStop/EstopPressedModal.tsx | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/app/src/organisms/EmergencyStop/EstopPressedModal.tsx b/app/src/organisms/EmergencyStop/EstopPressedModal.tsx index 5c80dab272a..4c573d4aa6a 100644 --- a/app/src/organisms/EmergencyStop/EstopPressedModal.tsx +++ b/app/src/organisms/EmergencyStop/EstopPressedModal.tsx @@ -26,7 +26,6 @@ import { StyledText } from '../../atoms/text' import { LegacyModal } from '../../molecules/LegacyModal' import { Modal } from '../../molecules/Modal' import { getIsOnDevice } from '../../redux/config' -import { DISENGAGED } from './constants' import type { ModalHeaderBaseProps, @@ -133,10 +132,7 @@ function DesktopModal({ }: EstopPressedModalProps): JSX.Element { const { t } = useTranslation('device_settings') const [isResuming, setIsResuming] = React.useState(false) - const { - acknowledgeEstopDisengage, - data, - } = useAcknowledgeEstopDisengageMutation() + const { acknowledgeEstopDisengage } = useAcknowledgeEstopDisengageMutation() const handleCloseModal = (): void => { if (setIsDismissedModal != null) { @@ -155,21 +151,18 @@ function DesktopModal({ } const handleClick: React.MouseEventHandler = (e): void => { + e.preventDefault() setIsResuming(true) acknowledgeEstopDisengage({ - onSuccess: () => {}, + onSuccess: () => { + closeModal() + }, onError: () => { setIsResuming(false) }, }) } - React.useEffect(() => { - if (data?.data.status === DISENGAGED) { - closeModal() - } - }, [data?.data.status, closeModal]) - return ( From 5d4845f6d3ca0d3a7cf6c5c665d6794e97cc29f5 Mon Sep 17 00:00:00 2001 From: Brian Arthur Cooper Date: Fri, 15 Dec 2023 14:10:40 -0500 Subject: [PATCH 12/32] fix(app): support addressable areas in manual intervention modal locations (#14220) Fix bug in which moves to manual moves to addressable areas would show nothing in the manual intervention modal Closes RQA-2114, Closes RQA-2115 --- .../utils/getLabwareDisplayLocation.ts | 5 +++ .../MoveLabwareInterventionContent.tsx | 7 ++++ .../__tests__/InterventionModal.test.tsx | 39 +++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/app/src/organisms/CommandText/utils/getLabwareDisplayLocation.ts b/app/src/organisms/CommandText/utils/getLabwareDisplayLocation.ts index aad564a1707..722fa731742 100644 --- a/app/src/organisms/CommandText/utils/getLabwareDisplayLocation.ts +++ b/app/src/organisms/CommandText/utils/getLabwareDisplayLocation.ts @@ -76,6 +76,11 @@ export function getLabwareDisplayLocation( adapter: adapterDisplayName, slot: adapter.location.slotName, }) + } else if ('addressableAreaName' in adapter.location) { + return t('adapter_in_slot', { + adapter: adapterDisplayName, + slot: adapter.location.addressableAreaName, + }) } else if ('moduleId' in adapter.location) { const moduleIdUnderAdapter = adapter.location.moduleId const moduleModel = robotSideAnalysis.modules.find( diff --git a/app/src/organisms/InterventionModal/MoveLabwareInterventionContent.tsx b/app/src/organisms/InterventionModal/MoveLabwareInterventionContent.tsx index 8d4925ce73b..958a296d657 100644 --- a/app/src/organisms/InterventionModal/MoveLabwareInterventionContent.tsx +++ b/app/src/organisms/InterventionModal/MoveLabwareInterventionContent.tsx @@ -255,6 +255,8 @@ function LabwareDisplayLocation( displayLocation = } else if ('slotName' in location) { displayLocation = + } else if ('addressableAreaName' in location) { + displayLocation = } else if ('moduleId' in location) { const moduleModel = getModuleModelFromRunData( protocolData, @@ -293,6 +295,11 @@ function LabwareDisplayLocation( adapter: adapterDisplayName, slot_name: adapter.location.slotName, }) + } else if ('addressableAreaName' in adapter.location) { + return t('adapter_in_slot', { + adapter: adapterDisplayName, + slot: adapter.location.addressableAreaName, + }) } else if ('moduleId' in adapter.location) { const moduleIdUnderAdapter = adapter.location.moduleId const moduleModel = protocolData.modules.find( diff --git a/app/src/organisms/InterventionModal/__tests__/InterventionModal.test.tsx b/app/src/organisms/InterventionModal/__tests__/InterventionModal.test.tsx index cd320a21058..4838082619d 100644 --- a/app/src/organisms/InterventionModal/__tests__/InterventionModal.test.tsx +++ b/app/src/organisms/InterventionModal/__tests__/InterventionModal.test.tsx @@ -107,6 +107,45 @@ describe('InterventionModal', () => { queryAllByText('D3') }) + it('renders a move labware intervention modal given a move labware command - between staging area slots', () => { + props = { + ...props, + command: { + id: 'mockMoveLabwareCommandId', + key: 'mockMoveLabwareCommandKey', + commandType: 'moveLabware', + params: { + labwareId: 'mockLabwareId', + newLocation: { + addressableAreaName: 'C4', + }, + strategy: 'manualMoveWithPause', + }, + startedAt: 'fake_timestamp', + completedAt: 'fake_timestamp', + createdAt: 'fake_timestamp', + status: 'succeeded', + }, + run: { + labware: [ + { + id: 'mockLabwareId', + displayName: 'mockLabwareInStagingArea', + location: { slotName: 'B4' }, + definitionUri: getLabwareDefURI(mockTipRackDefinition), + }, + ], + modules: [], + } as any, + } + const { getByText, queryAllByText } = render(props) + getByText('Move labware on Otie') + getByText('Labware name') + getByText('mockLabwareInStagingArea') + queryAllByText('B4') + queryAllByText('C4') + }) + it('renders a move labware intervention modal given a move labware command - module starting point', () => { props = { ...props, From f5a99be2d7783384d081ca6c8e1489013dce1146 Mon Sep 17 00:00:00 2001 From: Brent Hagen Date: Fri, 15 Dec 2023 14:21:23 -0500 Subject: [PATCH 13/32] refactor(components): adjust BaseDeck styling (#14219) adjusts BaseDeck styling of slot labels, staging area slot clip positioning, and colors to match design system deck map closes RAUT-902, RAUT-861 --- .../src/hardware-sim/BaseDeck/BaseDeck.tsx | 11 ++++--- .../BaseDeck/StagingAreaFixture.tsx | 32 +++++++++---------- .../BaseDeck/WasteChuteFixture.tsx | 6 ++-- .../BaseDeck/WasteChuteStagingAreaFixture.tsx | 14 ++++---- .../hardware-sim/DeckConfigurator/index.tsx | 2 +- .../__tests__/LocationIcon.test.tsx | 2 +- .../src/molecules/LocationIcon/index.tsx | 20 +++--------- .../src/components/DeckSetup/index.tsx | 3 +- 8 files changed, 41 insertions(+), 49 deletions(-) diff --git a/components/src/hardware-sim/BaseDeck/BaseDeck.tsx b/components/src/hardware-sim/BaseDeck/BaseDeck.tsx index 3de5dfb5071..2cf40694da5 100644 --- a/components/src/hardware-sim/BaseDeck/BaseDeck.tsx +++ b/components/src/hardware-sim/BaseDeck/BaseDeck.tsx @@ -71,6 +71,7 @@ interface BaseDeckProps { deckLayerBlocklist?: string[] showExpansion?: boolean lightFill?: string + mediumFill?: string darkFill?: string children?: React.ReactNode showSlotLabels?: boolean @@ -86,7 +87,8 @@ export function BaseDeck(props: BaseDeckProps): JSX.Element { modulesOnDeck = [], labwareOnDeck = [], lightFill = COLORS.light1, - darkFill = COLORS.darkGreyEnabled, + mediumFill = COLORS.grey2, + darkFill = COLORS.darkBlack70, deckLayerBlocklist = [], deckConfig, showExpansion = true, @@ -137,7 +139,7 @@ export function BaseDeck(props: BaseDeckProps): JSX.Element { {showSlotLabels ? ( 0 || wasteChuteStagingAreaFixtures.length > 0 @@ -177,7 +179,7 @@ export function BaseDeck(props: BaseDeckProps): JSX.Element { trashIconColor={lightFill} // TODO(bh, 2023-10-09): typeguard fixture location trashCutoutId={fixture.cutoutId as TrashCutoutId} - backgroundColor={darkFill} + backgroundColor={mediumFill} /> ))} @@ -187,8 +189,8 @@ export function BaseDeck(props: BaseDeckProps): JSX.Element { // TODO(bh, 2023-10-09): typeguard fixture location cutoutId={fixture.cutoutId as typeof WASTE_CHUTE_CUTOUT} deckDefinition={deckDef} - slotClipColor={darkFill} fixtureBaseColor={lightFill} + wasteChuteColor={mediumFill} /> ))} {wasteChuteStagingAreaFixtures.map(fixture => ( @@ -199,6 +201,7 @@ export function BaseDeck(props: BaseDeckProps): JSX.Element { deckDefinition={deckDef} slotClipColor={darkFill} fixtureBaseColor={lightFill} + wasteChuteColor={mediumFill} /> ))} diff --git a/components/src/hardware-sim/BaseDeck/StagingAreaFixture.tsx b/components/src/hardware-sim/BaseDeck/StagingAreaFixture.tsx index 107da94b8c2..600d4bfbd6f 100644 --- a/components/src/hardware-sim/BaseDeck/StagingAreaFixture.tsx +++ b/components/src/hardware-sim/BaseDeck/StagingAreaFixture.tsx @@ -54,10 +54,10 @@ export function StagingAreaFixture( , , - , - , - , - + , + , + , + ), cutoutB3: ( @@ -70,10 +70,10 @@ export function StagingAreaFixture( , , - , - , - , - + , + , + , + ), cutoutC3: ( @@ -86,10 +86,10 @@ export function StagingAreaFixture( , , - , - , - , - + , + , + , + ), cutoutD3: ( @@ -102,10 +102,10 @@ export function StagingAreaFixture( - , - , - , - + , + , + , + ), } diff --git a/components/src/hardware-sim/BaseDeck/WasteChuteFixture.tsx b/components/src/hardware-sim/BaseDeck/WasteChuteFixture.tsx index 9f562731b72..0928429edd7 100644 --- a/components/src/hardware-sim/BaseDeck/WasteChuteFixture.tsx +++ b/components/src/hardware-sim/BaseDeck/WasteChuteFixture.tsx @@ -21,7 +21,7 @@ interface WasteChuteFixtureProps extends React.SVGProps { deckDefinition: DeckDefinition moduleType?: ModuleType fixtureBaseColor?: React.SVGProps['fill'] - slotClipColor?: React.SVGProps['stroke'] + wasteChuteColor?: string showExtensions?: boolean } @@ -32,7 +32,7 @@ export function WasteChuteFixture( cutoutId, deckDefinition, fixtureBaseColor = COLORS.light1, - slotClipColor = COLORS.darkGreyEnabled, + wasteChuteColor = COLORS.grey2, ...restProps } = props @@ -60,7 +60,7 @@ export function WasteChuteFixture( fill={fixtureBaseColor} /> diff --git a/components/src/hardware-sim/BaseDeck/WasteChuteStagingAreaFixture.tsx b/components/src/hardware-sim/BaseDeck/WasteChuteStagingAreaFixture.tsx index 564db96c5fb..c75effcfa50 100644 --- a/components/src/hardware-sim/BaseDeck/WasteChuteStagingAreaFixture.tsx +++ b/components/src/hardware-sim/BaseDeck/WasteChuteStagingAreaFixture.tsx @@ -16,6 +16,7 @@ interface WasteChuteStagingAreaFixtureProps moduleType?: ModuleType fixtureBaseColor?: React.SVGProps['fill'] slotClipColor?: React.SVGProps['stroke'] + wasteChuteColor?: string showExtensions?: boolean } @@ -26,7 +27,8 @@ export function WasteChuteStagingAreaFixture( cutoutId, deckDefinition, fixtureBaseColor = COLORS.light1, - slotClipColor = COLORS.darkGreyEnabled, + slotClipColor = COLORS.darkBlack70, + wasteChuteColor = COLORS.grey2, ...restProps } = props @@ -53,13 +55,13 @@ export function WasteChuteStagingAreaFixture( d="M314.8,96.1h329.9c2.4,0,4.3-1.9,4.3-4.3V-5.6c0-2.4-1.9-4.3-4.3-4.3H314.8c-2.4,0-4.3,1.9-4.3,4.3v97.4C310.5,94.2,312.4,96.1,314.8,96.1z" fill={fixtureBaseColor} /> - , - , - , - + , + , + , + ) diff --git a/components/src/hardware-sim/DeckConfigurator/index.tsx b/components/src/hardware-sim/DeckConfigurator/index.tsx index 374c4d39ef9..f592b76d489 100644 --- a/components/src/hardware-sim/DeckConfigurator/index.tsx +++ b/components/src/hardware-sim/DeckConfigurator/index.tsx @@ -38,7 +38,7 @@ export function DeckConfigurator(props: DeckConfiguratorProps): JSX.Element { handleClickAdd, handleClickRemove, lightFill = COLORS.light1, - darkFill = COLORS.darkGreyEnabled, + darkFill = COLORS.darkBlackEnabled, readOnly = false, showExpansion = true, children, diff --git a/components/src/molecules/LocationIcon/__tests__/LocationIcon.test.tsx b/components/src/molecules/LocationIcon/__tests__/LocationIcon.test.tsx index 47f659d841d..785304a7c96 100644 --- a/components/src/molecules/LocationIcon/__tests__/LocationIcon.test.tsx +++ b/components/src/molecules/LocationIcon/__tests__/LocationIcon.test.tsx @@ -21,7 +21,7 @@ describe('LocationIcon', () => { it('should render the proper styles', () => { const [{ getByTestId }] = render(props) const locationIcon = getByTestId('LocationIcon_A1') - expect(locationIcon).toHaveStyle(`padding: ${SPACING.spacing2} 0.375rem`) + expect(locationIcon).toHaveStyle(`padding: ${SPACING.spacing4} 0.375rem`) expect(locationIcon).toHaveStyle('height: 2rem') expect(locationIcon).toHaveStyle('width: max-content') expect(locationIcon).toHaveStyle(`border: 2px solid ${COLORS.darkBlack100}`) diff --git a/components/src/molecules/LocationIcon/index.tsx b/components/src/molecules/LocationIcon/index.tsx index 42dc1e71632..4bb33b3ca34 100644 --- a/components/src/molecules/LocationIcon/index.tsx +++ b/components/src/molecules/LocationIcon/index.tsx @@ -4,13 +4,7 @@ import { css } from 'styled-components' import { Icon } from '../../icons' import { Flex, Text } from '../../primitives' import { ALIGN_CENTER } from '../../styles' -import { - BORDERS, - COLORS, - RESPONSIVENESS, - SPACING, - TYPOGRAPHY, -} from '../../ui-style-constants' +import { BORDERS, COLORS, SPACING, TYPOGRAPHY } from '../../ui-style-constants' import type { IconName } from '../../icons' import type { StyleProps } from '../../primitives' @@ -40,19 +34,13 @@ const LOCATION_ICON_STYLE = css<{ border: 2px solid ${props => props.color ?? COLORS.darkBlack100}; border-radius: ${BORDERS.borderRadiusSize3}; height: ${props => props.height ?? SPACING.spacing32}; - padding: ${SPACING.spacing2} 0.375rem; width: ${props => props.width ?? 'max-content'}; - @media ${RESPONSIVENESS.touchscreenMediaQuerySpecs} { - padding: ${SPACING.spacing4} - ${props => (props.slotName != null ? SPACING.spacing8 : SPACING.spacing6)}; - } + padding: ${SPACING.spacing4} + ${props => (props.slotName != null ? SPACING.spacing8 : SPACING.spacing6)}; ` const SLOT_NAME_TEXT_STYLE = css` - ${TYPOGRAPHY.pSemiBold} - @media ${RESPONSIVENESS.touchscreenMediaQuerySpecs} { - ${TYPOGRAPHY.smallBodyTextBold} - } + ${TYPOGRAPHY.smallBodyTextBold} ` export function LocationIcon({ diff --git a/protocol-designer/src/components/DeckSetup/index.tsx b/protocol-designer/src/components/DeckSetup/index.tsx index f826e695462..a69b2189f35 100644 --- a/protocol-designer/src/components/DeckSetup/index.tsx +++ b/protocol-designer/src/components/DeckSetup/index.tsx @@ -112,7 +112,7 @@ interface ContentsProps { } const lightFill = COLORS.light1 -const darkFill = COLORS.darkGreyEnabled +const darkFill = COLORS.darkBlack70 export const DeckSetupContents = (props: ContentsProps): JSX.Element => { const { @@ -631,7 +631,6 @@ export const DeckSetup = (): JSX.Element => { key={fixture.id} cutoutId={fixture.location as typeof WASTE_CHUTE_CUTOUT} deckDefinition={deckDef} - slotClipColor={darkFill} fixtureBaseColor={lightFill} /> ))} From 4da8206593bca58e80f4beab45b1aff7eac136ee Mon Sep 17 00:00:00 2001 From: Jamey H Date: Fri, 15 Dec 2023 14:48:21 -0500 Subject: [PATCH 14/32] fix(app): ODD - Remove dismisscurrentrun on protocol cancel button (#14218) * fix(app): remove dismissCurrentRun when run is cancelled on ODD Mirror the desktop app. Dismissing the run context on run cancel means drop tip will not work. --- .../__tests__/ProtocolRunHeader.test.tsx | 9 --------- .../RunningProtocol/ConfirmCancelRunModal.tsx | 12 ++---------- .../__tests__/ConfirmCancelRunModal.test.tsx | 13 ++----------- 3 files changed, 4 insertions(+), 30 deletions(-) diff --git a/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx b/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx index aa65ae218e0..cad5de28172 100644 --- a/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx +++ b/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx @@ -22,7 +22,6 @@ import { useRunQuery, useModulesQuery, usePipettesQuery, - useDismissCurrentRunMutation, useEstopQuery, useDoorQuery, useInstrumentsQuery, @@ -187,9 +186,6 @@ const mockUseModulesQuery = useModulesQuery as jest.MockedFunction< const mockUsePipettesQuery = usePipettesQuery as jest.MockedFunction< typeof usePipettesQuery > -const mockUseDismissCurrentRunMutation = useDismissCurrentRunMutation as jest.MockedFunction< - typeof useDismissCurrentRunMutation -> const mockConfirmCancelModal = ConfirmCancelModal as jest.MockedFunction< typeof ConfirmCancelModal > @@ -396,11 +392,6 @@ describe('ProtocolRunHeader', () => { .mockReturnValue({ data: { data: mockIdleUnstartedRun }, } as UseQueryResult) - when(mockUseDismissCurrentRunMutation) - .calledWith() - .mockReturnValue({ - dismissCurrentRun: jest.fn(), - } as any) when(mockUseProtocolDetailsForRun) .calledWith(RUN_ID) .mockReturnValue(PROTOCOL_DETAILS) diff --git a/app/src/organisms/OnDeviceDisplay/RunningProtocol/ConfirmCancelRunModal.tsx b/app/src/organisms/OnDeviceDisplay/RunningProtocol/ConfirmCancelRunModal.tsx index 1b6996ba885..771417f33c3 100644 --- a/app/src/organisms/OnDeviceDisplay/RunningProtocol/ConfirmCancelRunModal.tsx +++ b/app/src/organisms/OnDeviceDisplay/RunningProtocol/ConfirmCancelRunModal.tsx @@ -10,10 +10,7 @@ import { Flex, SPACING, } from '@opentrons/components' -import { - useStopRunMutation, - useDismissCurrentRunMutation, -} from '@opentrons/react-api-client' +import { useStopRunMutation } from '@opentrons/react-api-client' import { StyledText } from '../../../atoms/text' import { SmallButton } from '../../../atoms/buttons' @@ -40,10 +37,6 @@ export function ConfirmCancelRunModal({ }: ConfirmCancelRunModalProps): JSX.Element { const { t } = useTranslation(['run_details', 'shared']) const { stopRun } = useStopRunMutation() - const { - dismissCurrentRun, - isLoading: isDismissing, - } = useDismissCurrentRunMutation() const runStatus = useRunStatus(runId) const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId) const history = useHistory() @@ -68,7 +61,6 @@ export function ConfirmCancelRunModal({ React.useEffect(() => { if (runStatus === RUN_STATUS_STOPPED) { trackProtocolRunEvent({ name: ANALYTICS_PROTOCOL_RUN_CANCEL }) - dismissCurrentRun(runId) if (!isActiveRun) { if (protocolId != null) { history.push(`/protocols/${protocolId}`) @@ -79,7 +71,7 @@ export function ConfirmCancelRunModal({ } }, [runStatus]) - return isCanceling || isDismissing ? ( + return isCanceling ? ( ) : ( { getByText('Cancel run') }) - it('shoudler render the canceling run modal when run is dismissing', () => { - mockUseDismissCurrentRunMutation.mockReturnValue({ - dismissCurrentRun: mockDismissCurrentRun, - isLoading: true, - } as any) - const [{ getByText }] = render(props) - getByText('mock CancelingRunModal') - }) - it('when tapping go back, the mock function is called', () => { const [{ getByText }] = render(props) const button = getByText('Go back') @@ -147,7 +138,7 @@ describe('ConfirmCancelRunModal', () => { .mockReturnValue(RUN_STATUS_STOPPED) render(props) - expect(mockDismissCurrentRun).toHaveBeenCalled() + expect(mockDismissCurrentRun).not.toHaveBeenCalled() expect(mockTrackProtocolRunEvent).toHaveBeenCalled() }) @@ -161,7 +152,7 @@ describe('ConfirmCancelRunModal', () => { .mockReturnValue(RUN_STATUS_STOPPED) render(props) - expect(mockDismissCurrentRun).toHaveBeenCalled() + expect(mockDismissCurrentRun).not.toHaveBeenCalled() expect(mockTrackProtocolRunEvent).toHaveBeenCalled() expect(mockPush).toHaveBeenCalledWith('/protocols') }) From 8c82d7c1ac5e749edf5f1a1e4cc64e94ac7453bf Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Fri, 15 Dec 2023 15:03:04 -0500 Subject: [PATCH 15/32] fix(api): allow double remove tip (#14224) This seems like a logic problem with how we track tip state, but removing this check (which is really a nebulous "might be using it wrong" check) we fix some incorrect errors in the drop tip wizard. --- .../opentrons/hardware_control/instruments/ot2/pipette.py | 1 - .../opentrons/hardware_control/instruments/ot3/pipette.py | 1 - api/tests/opentrons/hardware_control/test_pipette.py | 6 ++---- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/api/src/opentrons/hardware_control/instruments/ot2/pipette.py b/api/src/opentrons/hardware_control/instruments/ot2/pipette.py index 1a4b82e5f25..f69d11f82b1 100644 --- a/api/src/opentrons/hardware_control/instruments/ot2/pipette.py +++ b/api/src/opentrons/hardware_control/instruments/ot2/pipette.py @@ -510,7 +510,6 @@ def remove_tip(self) -> None: Remove the tip from the pipette (effectively updates the pipette's critical point) """ - assert self.has_tip self._has_tip = False self._current_tip_length = 0.0 diff --git a/api/src/opentrons/hardware_control/instruments/ot3/pipette.py b/api/src/opentrons/hardware_control/instruments/ot3/pipette.py index 7ec78cfbe80..2d36460ca69 100644 --- a/api/src/opentrons/hardware_control/instruments/ot3/pipette.py +++ b/api/src/opentrons/hardware_control/instruments/ot3/pipette.py @@ -484,7 +484,6 @@ def remove_tip(self) -> None: Remove the tip from the pipette (effectively updates the pipette's critical point) """ - assert self.has_tip_length self._current_tip_length = 0.0 self._has_tip_length = False diff --git a/api/tests/opentrons/hardware_control/test_pipette.py b/api/tests/opentrons/hardware_control/test_pipette.py index d02fedf88c3..b6224a4e3dd 100644 --- a/api/tests/opentrons/hardware_control/test_pipette.py +++ b/api/tests/opentrons/hardware_control/test_pipette.py @@ -85,8 +85,7 @@ def test_tip_tracking( model: Union[str, pipette_definition.PipetteModelVersionType], ) -> None: hw_pipette = pipette_builder(model) - with pytest.raises(AssertionError): - hw_pipette.remove_tip() + hw_pipette.remove_tip() assert not hw_pipette.has_tip tip_length = 25.0 hw_pipette.add_tip(tip_length) @@ -95,8 +94,7 @@ def test_tip_tracking( hw_pipette.add_tip(tip_length) hw_pipette.remove_tip() assert not hw_pipette.has_tip - with pytest.raises(AssertionError): - hw_pipette.remove_tip() + hw_pipette.remove_tip() @pytest.mark.parametrize( From 3135a08a1f8084acbf91a25d41f20faa5f5b60df Mon Sep 17 00:00:00 2001 From: koji Date: Fri, 15 Dec 2023 15:26:50 -0500 Subject: [PATCH 16/32] fix(app): allow users to use the same name after device reset (#14222) * fix(app): allow users to use the same name after clearing all data --- app/src/pages/OnDeviceDisplay/NameRobot.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/pages/OnDeviceDisplay/NameRobot.tsx b/app/src/pages/OnDeviceDisplay/NameRobot.tsx index fea706a90d0..7e778652457 100644 --- a/app/src/pages/OnDeviceDisplay/NameRobot.tsx +++ b/app/src/pages/OnDeviceDisplay/NameRobot.tsx @@ -59,6 +59,7 @@ export function NameRobot(): JSX.Element { const history = useHistory() const trackEvent = useTrackEvent() const localRobot = useSelector(getLocalRobot) + const ipAddress = localRobot?.ip const previousName = localRobot?.name != null ? localRobot.name : null const [name, setName] = React.useState('') const [newName, setNewName] = React.useState('') @@ -105,7 +106,7 @@ export function NameRobot(): JSX.Element { } if ( [...connectableRobots, ...reachableRobots].some( - robot => newName === robot.name + robot => newName === robot.name && robot.ip !== ipAddress ) ) { errors.newRobotName = t('name_rule_error_exist') From 9a05d1f0526c6273cf6643aeb7a7966ee457363b Mon Sep 17 00:00:00 2001 From: CaseyBatten Date: Fri, 15 Dec 2023 16:45:48 -0500 Subject: [PATCH 17/32] fix(api): Flex 2 15 api fixed trash auto tipdrop support (#14225) ensures that all OT2 protocols auto drop tips regardless of version, and that only Flex protocols 2.15 and prior auto drop tips --- .../execution/hardware_stopper.py | 41 ++++--- .../execution/test_hardware_stopper.py | 111 +++++++++++++++++- .../robot_server/runs/engine_store.py | 3 - 3 files changed, 134 insertions(+), 21 deletions(-) diff --git a/api/src/opentrons/protocol_engine/execution/hardware_stopper.py b/api/src/opentrons/protocol_engine/execution/hardware_stopper.py index 044b1f83daf..28eacd7525b 100644 --- a/api/src/opentrons/protocol_engine/execution/hardware_stopper.py +++ b/api/src/opentrons/protocol_engine/execution/hardware_stopper.py @@ -74,14 +74,23 @@ async def _drop_tip(self) -> None: await self._home_everything_except_plungers() - # OT-2 Will only ever use the Fixed Trash Addressable Area - if self._state_store.config.robot_type == "OT-2 Standard": - for pipette_id, tip in attached_tips: - try: + for pipette_id, tip in attached_tips: + try: + if self._state_store.labware.get_fixed_trash_id() == FIXED_TRASH_ID: + # OT-2 and Flex 2.15 protocols will default to the Fixed Trash Labware + await self._tip_handler.add_tip(pipette_id=pipette_id, tip=tip) + await self._movement_handler.move_to_well( + pipette_id=pipette_id, + labware_id=FIXED_TRASH_ID, + well_name="A1", + ) + await self._tip_handler.drop_tip( + pipette_id=pipette_id, + home_after=False, + ) + elif self._state_store.config.robot_type == "OT-2 Standard": + # API 2.16 and above OT2 protocols use addressable areas await self._tip_handler.add_tip(pipette_id=pipette_id, tip=tip) - # TODO: Add ability to drop tip onto custom trash as well. - # if API is 2.15 and below aka is should_have_fixed_trash - await self._movement_handler.move_to_addressable_area( pipette_id=pipette_id, addressable_area_name="fixedTrash", @@ -90,21 +99,19 @@ async def _drop_tip(self) -> None: speed=None, minimum_z_height=None, ) - await self._tip_handler.drop_tip( pipette_id=pipette_id, home_after=False, ) + else: + log.debug( + "Flex Protocols API Version 2.16 and beyond do not support automatic tip dropping at this time." + ) - except HwPipetteNotAttachedError: - # this will happen normally during protocol analysis, but - # should not happen during an actual run - log.debug(f"Pipette ID {pipette_id} no longer attached.") - - else: - log.debug( - "Flex protocols do not support automatic tip dropping at this time." - ) + except HwPipetteNotAttachedError: + # this will happen normally during protocol analysis, but + # should not happen during an actual run + log.debug(f"Pipette ID {pipette_id} no longer attached.") async def do_halt(self, disengage_before_stopping: bool = False) -> None: """Issue a halt signal to the hardware API. diff --git a/api/tests/opentrons/protocol_engine/execution/test_hardware_stopper.py b/api/tests/opentrons/protocol_engine/execution/test_hardware_stopper.py index 891c7e4d3ae..537fd07613c 100644 --- a/api/tests/opentrons/protocol_engine/execution/test_hardware_stopper.py +++ b/api/tests/opentrons/protocol_engine/execution/test_hardware_stopper.py @@ -15,7 +15,12 @@ TipHandler, HardwareStopper, ) -from opentrons.protocol_engine.types import MotorAxis, TipGeometry, PostRunHardwareState +from opentrons.protocol_engine.types import ( + MotorAxis, + TipGeometry, + PostRunHardwareState, + AddressableOffsetVector, +) if TYPE_CHECKING: from opentrons.hardware_control.ot3api import OT3API @@ -229,3 +234,107 @@ async def test_hardware_stopping_sequence_with_gripper( ), await ot3_hardware_api.stop(home_after=True), ) + + +@pytest.mark.ot3_only +async def test_hardware_stopping_sequence_with_fixed_trash( + decoy: Decoy, + state_store: StateStore, + ot3_hardware_api: OT3API, + movement: MovementHandler, + mock_tip_handler: TipHandler, +) -> None: + """It should stop the hardware, and home the robot. Flex no longer performs automatic drop tip.""" + subject = HardwareStopper( + hardware_api=ot3_hardware_api, + state_store=state_store, + movement=movement, + tip_handler=mock_tip_handler, + ) + decoy.when(state_store.pipettes.get_all_attached_tips()).then_return( + [ + ("pipette-id", TipGeometry(length=1.0, volume=2.0, diameter=3.0)), + ] + ) + decoy.when(state_store.labware.get_fixed_trash_id()).then_return("fixedTrash") + decoy.when(state_store.config.use_virtual_gripper).then_return(False) + decoy.when(ot3_hardware_api.has_gripper()).then_return(True) + + await subject.do_stop_and_recover( + drop_tips_after_run=True, + post_run_hardware_state=PostRunHardwareState.HOME_AND_STAY_ENGAGED, + ) + + decoy.verify( + await ot3_hardware_api.stop(home_after=False), + await ot3_hardware_api.home_z(mount=OT3Mount.GRIPPER), + await movement.home( + axes=[MotorAxis.X, MotorAxis.Y, MotorAxis.LEFT_Z, MotorAxis.RIGHT_Z] + ), + await mock_tip_handler.add_tip( + pipette_id="pipette-id", + tip=TipGeometry(length=1.0, volume=2.0, diameter=3.0), + ), + await movement.move_to_well( + pipette_id="pipette-id", + labware_id="fixedTrash", + well_name="A1", + ), + await mock_tip_handler.drop_tip( + pipette_id="pipette-id", + home_after=False, + ), + await ot3_hardware_api.stop(home_after=True), + ) + + +async def test_hardware_stopping_sequence_with_OT2_addressable_area( + decoy: Decoy, + state_store: StateStore, + hardware_api: HardwareAPI, + movement: MovementHandler, + mock_tip_handler: TipHandler, +) -> None: + """It should stop the hardware, and home the robot. Flex no longer performs automatic drop tip.""" + subject = HardwareStopper( + hardware_api=hardware_api, + state_store=state_store, + movement=movement, + tip_handler=mock_tip_handler, + ) + decoy.when(state_store.pipettes.get_all_attached_tips()).then_return( + [ + ("pipette-id", TipGeometry(length=1.0, volume=2.0, diameter=3.0)), + ] + ) + decoy.when(state_store.config.robot_type).then_return("OT-2 Standard") + decoy.when(state_store.config.use_virtual_gripper).then_return(False) + + await subject.do_stop_and_recover( + drop_tips_after_run=True, + post_run_hardware_state=PostRunHardwareState.HOME_AND_STAY_ENGAGED, + ) + + decoy.verify( + await hardware_api.stop(home_after=False), + await movement.home( + axes=[MotorAxis.X, MotorAxis.Y, MotorAxis.LEFT_Z, MotorAxis.RIGHT_Z] + ), + await mock_tip_handler.add_tip( + pipette_id="pipette-id", + tip=TipGeometry(length=1.0, volume=2.0, diameter=3.0), + ), + await movement.move_to_addressable_area( + pipette_id="pipette-id", + addressable_area_name="fixedTrash", + offset=AddressableOffsetVector(x=0, y=0, z=0), + force_direct=False, + speed=None, + minimum_z_height=None, + ), + await mock_tip_handler.drop_tip( + pipette_id="pipette-id", + home_after=False, + ), + await hardware_api.stop(home_after=True), + ) diff --git a/robot-server/robot_server/runs/engine_store.py b/robot-server/robot_server/runs/engine_store.py index b3adf76306a..350d5bc694c 100644 --- a/robot-server/robot_server/runs/engine_store.py +++ b/robot-server/robot_server/runs/engine_store.py @@ -188,9 +188,6 @@ async def create( post_run_hardware_state = PostRunHardwareState.HOME_AND_STAY_ENGAGED drop_tips_after_run = True - if self._robot_type == "OT-3 Standard": - post_run_hardware_state = PostRunHardwareState.HOME_AND_STAY_ENGAGED - drop_tips_after_run = False runner = create_protocol_runner( protocol_engine=engine, From 32605a5f3177accaa27403dd849fab2d08e7468e Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Fri, 15 Dec 2023 17:40:40 -0500 Subject: [PATCH 18/32] fix(api): center only active column on 12-well (#14226) We have a system to center pipettes appropriately on the wells of labware that span multiple "traditional" wells. For instance, in a 12-well reservoir, we need to move the center of an eight channel pipette to the center of a given well of the 12-well reservoir. However, that code was moving the center of _any_ pipette to the center of a 12-well reservoir, inclunding the center of the 96-channel, which is definitely not right. Instead, what we want to do is move the center of the "active" column (the column that contains the active nozzle, which is the nozzle that is moved when a caller specifies a NOZZLE critical point) to the center of the well. And we only want to do this on labware that have roughly one well per column. The fix is in two parts. First, a Y_CENTER critical point needs to exist. This critical point is aligned in the X direction with the active nozzle of the pipette, but is halfway between the Y positions of the backmost and frontmost nozzles in the column containing the active nozzle of the currently active nozzlemap - so, on an eight channel with a full map it would be the same as XY_CENTER; on a single, an eight channel with a single nozzle map, or a 96 with a single nozzle map, it will be the same as NOZZLE (and the same as XY_CENTER); and on an eight channel or 96 with a partial-column map it will be halfway up the partial. On a 96 with a rectangular map (i.e. either full or quadrant) it will be halfway between the back left and front left nozzles. Next, we have to actually use that critical point. The engine would check for a labware to have the centerMultiChannelOnWells quirk and emit XY_CENTER for both source and destination if it was found; we now also consider how many wells the labware has if it has centerMultiChannelOnWells and (somewhat hackily, sadly) emit - Y_CENTER, if the labware has the centerMultiChannelOnWells quirk and 1 < wells < 96 - XY_CENTER, if the labware has the centerMultiChannelOnWells quirk and wells == 1, wells >= 96 This should probably change eventually into a new quirk called like centerActiveColumnOnWells or something but that's for a different time. Closes RQA-2118 --- .../hardware_control/nozzle_manager.py | 9 + api/src/opentrons/hardware_control/types.py | 10 + .../protocol_engine/state/labware.py | 22 ++ .../opentrons/protocol_engine/state/motion.py | 35 ++-- .../instruments/test_nozzle_manager.py | 110 ++++++---- .../protocol_engine/state/test_motion_view.py | 193 ++++++++++++++++-- 6 files changed, 310 insertions(+), 69 deletions(-) diff --git a/api/src/opentrons/hardware_control/nozzle_manager.py b/api/src/opentrons/hardware_control/nozzle_manager.py index 4841a1fdee8..c3b8c63fc3a 100644 --- a/api/src/opentrons/hardware_control/nozzle_manager.py +++ b/api/src/opentrons/hardware_control/nozzle_manager.py @@ -140,6 +140,13 @@ def xy_center_offset(self) -> Point: difference[0] / 2, difference[1] / 2, 0 ) + @property + def y_center_offset(self) -> Point: + """The position in the center of the primary column of the map.""" + front_left = next(reversed(list(self.rows.values())))[0] + difference = self.map_store[front_left] - self.map_store[self.back_left] + return self.map_store[self.back_left] + Point(0, difference[1] / 2, 0) + @property def front_nozzle_offset(self) -> Point: """The offset for the front_left nozzle.""" @@ -319,6 +326,8 @@ def critical_point_with_tip_length( ) -> Point: if cp_override == CriticalPoint.XY_CENTER: current_nozzle = self._current_nozzle_configuration.xy_center_offset + elif cp_override == CriticalPoint.Y_CENTER: + current_nozzle = self._current_nozzle_configuration.y_center_offset elif cp_override == CriticalPoint.FRONT_NOZZLE: current_nozzle = self._current_nozzle_configuration.front_nozzle_offset else: diff --git a/api/src/opentrons/hardware_control/types.py b/api/src/opentrons/hardware_control/types.py index 398d7eeaed3..9cd8c6b758e 100644 --- a/api/src/opentrons/hardware_control/types.py +++ b/api/src/opentrons/hardware_control/types.py @@ -505,6 +505,16 @@ class CriticalPoint(enum.Enum): back calibration pin slot. """ + Y_CENTER = enum.auto() + """ + Y_CENTER means the critical point under consideration is at the same X + coordinate as the default nozzle point (i.e. TIP | NOZZLE | FRONT_NOZZLE) + but halfway in between the Y axis bounding box of the pipette - it is the + XY center of the first column in the pipette. It's really only relevant for + the 96; it will produce the same position as XY_CENTER on an eight or one + channel pipette. + """ + class ExecutionState(enum.Enum): RUNNING = enum.auto() diff --git a/api/src/opentrons/protocol_engine/state/labware.py b/api/src/opentrons/protocol_engine/state/labware.py index d1e0e207b7b..2b6f498ac50 100644 --- a/api/src/opentrons/protocol_engine/state/labware.py +++ b/api/src/opentrons/protocol_engine/state/labware.py @@ -381,6 +381,28 @@ def get_quirks(self, labware_id: str) -> List[str]: definition = self.get_definition(labware_id) return definition.parameters.quirks or [] + def get_should_center_column_on_target_well(self, labware_id: str) -> bool: + """True if a pipette moving to this labware should center its active column on the target. + + This is true for labware that have wells spanning entire columns. + """ + has_quirk = self.get_has_quirk(labware_id, "centerMultichannelOnWells") + return has_quirk and ( + len(self.get_definition(labware_id).wells) > 1 + and len(self.get_definition(labware_id).wells) < 96 + ) + + def get_should_center_pipette_on_target_well(self, labware_id: str) -> bool: + """True if a pipette moving to a well of this labware should center its body on the target. + + This is true for 1-well reservoirs no matter the pipette, and for large plates. + """ + has_quirk = self.get_has_quirk(labware_id, "centerMultichannelOnWells") + return has_quirk and ( + len(self.get_definition(labware_id).wells) == 1 + or len(self.get_definition(labware_id).wells) >= 96 + ) + def get_well_definition( self, labware_id: str, diff --git a/api/src/opentrons/protocol_engine/state/motion.py b/api/src/opentrons/protocol_engine/state/motion.py index 4613b69e5b2..8735b5d492e 100644 --- a/api/src/opentrons/protocol_engine/state/motion.py +++ b/api/src/opentrons/protocol_engine/state/motion.py @@ -69,16 +69,19 @@ def get_pipette_location( critical_point = None # if the pipette was last used to move to a labware that requires - # centering, set the critical point to XY_CENTER + # centering, set the critical point to the appropriate center if ( isinstance(current_location, CurrentWell) and current_location.pipette_id == pipette_id - and self._labware.get_has_quirk( - current_location.labware_id, - "centerMultichannelOnWells", - ) ): - critical_point = CriticalPoint.XY_CENTER + if self._labware.get_should_center_column_on_target_well( + current_location.labware_id + ): + critical_point = CriticalPoint.Y_CENTER + elif self._labware.get_should_center_pipette_on_target_well( + current_location.labware_id + ): + critical_point = CriticalPoint.XY_CENTER return PipetteLocationData(mount=mount, critical_point=critical_point) def get_movement_waypoints_to_well( @@ -97,17 +100,17 @@ def get_movement_waypoints_to_well( """Calculate waypoints to a destination that's specified as a well.""" location = current_well or self._pipettes.get_current_location() - center_destination = self._labware.get_has_quirk( - labware_id, - "centerMultichannelOnWells", - ) + destination_cp: Optional[CriticalPoint] = None + if self._labware.get_should_center_column_on_target_well(labware_id): + destination_cp = CriticalPoint.Y_CENTER + elif self._labware.get_should_center_pipette_on_target_well(labware_id): + destination_cp = CriticalPoint.XY_CENTER destination = self._geometry.get_well_position( labware_id, well_name, well_location, ) - destination_cp = CriticalPoint.XY_CENTER if center_destination else None move_type = move_types.get_move_type_to_well( pipette_id, labware_id, well_name, location, force_direct @@ -306,12 +309,12 @@ def get_touch_tip_waypoints( positions = move_types.get_edge_point_list( center_point, x_offset, y_offset, edge_path_type ) + critical_point: Optional[CriticalPoint] = None - critical_point = ( - CriticalPoint.XY_CENTER - if self._labware.get_has_quirk(labware_id, "centerMultichannelOnWells") - else None - ) + if self._labware.get_should_center_column_on_target_well(labware_id): + critical_point = CriticalPoint.Y_CENTER + elif self._labware.get_should_center_pipette_on_target_well(labware_id): + critical_point = CriticalPoint.XY_CENTER return [ motion_planning.Waypoint(position=p, critical_point=critical_point) diff --git a/api/tests/opentrons/hardware_control/instruments/test_nozzle_manager.py b/api/tests/opentrons/hardware_control/instruments/test_nozzle_manager.py index ca963355cb2..b87c542b07e 100644 --- a/api/tests/opentrons/hardware_control/instruments/test_nozzle_manager.py +++ b/api/tests/opentrons/hardware_control/instruments/test_nozzle_manager.py @@ -104,6 +104,7 @@ def test_single_pipette_map_geometry( def test_map_geometry(nozzlemap: nozzle_manager.NozzleMap) -> None: assert nozzlemap.xy_center_offset == Point(*config.nozzle_map["A1"]) + assert nozzlemap.y_center_offset == Point(*config.nozzle_map["A1"]) assert nozzlemap.front_nozzle_offset == Point(*config.nozzle_map["A1"]) assert nozzlemap.starting_nozzle_offset == Point(*config.nozzle_map["A1"]) @@ -228,6 +229,22 @@ def test_map_entries( test_map_entries(subject.current_configuration, ["C1", "D1", "E1", "F1"]) +def assert_offset_in_center_of( + offset: Point, between: Union[Tuple[str, str], str], config: PipetteConfigurations +) -> None: + if isinstance(between, str): + assert offset == Point(*config.nozzle_map[between]) + else: + assert ( + offset + == ( + Point(*config.nozzle_map[between[0]]) + + Point(*config.nozzle_map[between[1]]) + ) + * 0.5 + ) + + @pytest.mark.parametrize( "pipette_details", [ @@ -251,40 +268,42 @@ def test_map_geometry( front_nozzle: str, starting_nozzle: str, xy_center_in_center_of: Union[Tuple[str, str], str], + y_center_in_center_of: Union[Tuple[str, str], str], ) -> None: - if isinstance(xy_center_in_center_of, str): - assert nozzlemap.xy_center_offset == Point( - *config.nozzle_map[xy_center_in_center_of] - ) - else: - assert nozzlemap.xy_center_offset == ( - ( - Point(*config.nozzle_map[xy_center_in_center_of[0]]) - + Point(*config.nozzle_map[xy_center_in_center_of[1]]) - ) - * 0.5 - ) + assert_offset_in_center_of( + nozzlemap.xy_center_offset, xy_center_in_center_of, config + ) + assert_offset_in_center_of( + nozzlemap.y_center_offset, y_center_in_center_of, config + ) + assert nozzlemap.front_nozzle_offset == Point(*config.nozzle_map[front_nozzle]) assert nozzlemap.starting_nozzle_offset == Point( *config.nozzle_map[starting_nozzle] ) - test_map_geometry(subject.current_configuration, "H1", "A1", ("A1", "H1")) + test_map_geometry( + subject.current_configuration, "H1", "A1", ("A1", "H1"), ("A1", "H1") + ) subject.update_nozzle_configuration("A1", "A1", "A1") - test_map_geometry(subject.current_configuration, "A1", "A1", "A1") + test_map_geometry(subject.current_configuration, "A1", "A1", "A1", "A1") subject.update_nozzle_configuration("D1", "D1", "D1") - test_map_geometry(subject.current_configuration, "D1", "D1", "D1") + test_map_geometry(subject.current_configuration, "D1", "D1", "D1", "D1") subject.update_nozzle_configuration("C1", "G1", "C1") - test_map_geometry(subject.current_configuration, "G1", "C1", "E1") + test_map_geometry(subject.current_configuration, "G1", "C1", "E1", "E1") subject.update_nozzle_configuration("E1", "H1", "E1") - test_map_geometry(subject.current_configuration, "H1", "E1", ("E1", "H1")) + test_map_geometry( + subject.current_configuration, "H1", "E1", ("E1", "H1"), ("E1", "H1") + ) subject.reset_to_default_configuration() - test_map_geometry(subject.current_configuration, "H1", "A1", ("A1", "H1")) + test_map_geometry( + subject.current_configuration, "H1", "A1", ("A1", "H1"), ("A1", "H1") + ) @pytest.mark.parametrize( @@ -790,48 +809,59 @@ def test_map_geometry( nozzlemap: nozzle_manager.NozzleMap, starting_nozzle: str, front_nozzle: str, - center_between: Union[str, Tuple[str, str]], + xy_center_between: Union[str, Tuple[str, str]], + y_center_between: Union[str, Tuple[str, str]], ) -> None: - if isinstance(center_between, str): - assert nozzlemap.xy_center_offset == Point( - *config.nozzle_map[center_between] - ) - else: - assert ( - nozzlemap.xy_center_offset - == ( - Point(*config.nozzle_map[center_between[0]]) - + Point(*config.nozzle_map[center_between[1]]) - ) - * 0.5 - ) + assert_offset_in_center_of( + nozzlemap.xy_center_offset, xy_center_between, config + ) + assert_offset_in_center_of(nozzlemap.y_center_offset, y_center_between, config) assert nozzlemap.front_nozzle_offset == Point(*config.nozzle_map[front_nozzle]) assert nozzlemap.starting_nozzle_offset == Point( *config.nozzle_map[starting_nozzle] ) - test_map_geometry(config, subject.current_configuration, "A1", "H1", ("A1", "H12")) + test_map_geometry( + config, subject.current_configuration, "A1", "H1", ("A1", "H12"), ("A1", "H1") + ) subject.update_nozzle_configuration("A1", "H1") - test_map_geometry(config, subject.current_configuration, "A1", "H1", ("A1", "H1")) + test_map_geometry( + config, subject.current_configuration, "A1", "H1", ("A1", "H1"), ("A1", "H1") + ) subject.update_nozzle_configuration("A12", "H12") test_map_geometry( - config, subject.current_configuration, "A12", "H12", ("A12", "H12") + config, + subject.current_configuration, + "A12", + "H12", + ("A12", "H12"), + ("A12", "H12"), ) subject.update_nozzle_configuration("A1", "A12") - test_map_geometry(config, subject.current_configuration, "A1", "A1", ("A1", "A12")) + test_map_geometry( + config, subject.current_configuration, "A1", "A1", ("A1", "A12"), "A1" + ) subject.update_nozzle_configuration("H1", "H12") - test_map_geometry(config, subject.current_configuration, "H1", "H1", ("H1", "H12")) + test_map_geometry( + config, subject.current_configuration, "H1", "H1", ("H1", "H12"), "H1" + ) subject.update_nozzle_configuration("A1", "D6") - test_map_geometry(config, subject.current_configuration, "A1", "D1", ("A1", "D6")) + test_map_geometry( + config, subject.current_configuration, "A1", "D1", ("A1", "D6"), ("A1", "D1") + ) subject.update_nozzle_configuration("E7", "H12") - test_map_geometry(config, subject.current_configuration, "E7", "H7", ("E7", "H12")) + test_map_geometry( + config, subject.current_configuration, "E7", "H7", ("E7", "H12"), ("E7", "H7") + ) subject.update_nozzle_configuration("C4", "D5") - test_map_geometry(config, subject.current_configuration, "C4", "D4", ("C4", "D5")) + test_map_geometry( + config, subject.current_configuration, "C4", "D4", ("C4", "D5"), ("C4", "D4") + ) diff --git a/api/tests/opentrons/protocol_engine/state/test_motion_view.py b/api/tests/opentrons/protocol_engine/state/test_motion_view.py index 9e9920a7d3f..750eb7b9f4b 100644 --- a/api/tests/opentrons/protocol_engine/state/test_motion_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_motion_view.py @@ -99,13 +99,13 @@ def test_get_pipette_location_with_no_current_location( assert result == PipetteLocationData(mount=MountType.LEFT, critical_point=None) -def test_get_pipette_location_with_current_location_with_quirks( +def test_get_pipette_location_with_current_location_with_y_center( decoy: Decoy, labware_view: LabwareView, pipette_view: PipetteView, subject: MotionView, ) -> None: - """It should return cp=XY_CENTER if location labware has center quirk.""" + """It should return cp=Y_CENTER if location labware requests.""" decoy.when(pipette_view.get_current_location()).then_return( CurrentWell(pipette_id="pipette-id", labware_id="reservoir-id", well_name="A1") ) @@ -119,9 +119,41 @@ def test_get_pipette_location_with_current_location_with_quirks( ) decoy.when( - labware_view.get_has_quirk( + labware_view.get_should_center_column_on_target_well( + "reservoir-id", + ) + ).then_return(True) + + result = subject.get_pipette_location("pipette-id") + + assert result == PipetteLocationData( + mount=MountType.RIGHT, + critical_point=CriticalPoint.Y_CENTER, + ) + + +def test_get_pipette_location_with_current_location_with_xy_center( + decoy: Decoy, + labware_view: LabwareView, + pipette_view: PipetteView, + subject: MotionView, +) -> None: + """It should return cp=XY_CENTER if location labware requests.""" + decoy.when(pipette_view.get_current_location()).then_return( + CurrentWell(pipette_id="pipette-id", labware_id="reservoir-id", well_name="A1") + ) + + decoy.when(pipette_view.get("pipette-id")).then_return( + LoadedPipette( + id="pipette-id", + mount=MountType.RIGHT, + pipetteName=PipetteNameType.P300_SINGLE, + ) + ) + + decoy.when( + labware_view.get_should_center_pipette_on_target_well( "reservoir-id", - "centerMultichannelOnWells", ) ).then_return(True) @@ -157,9 +189,14 @@ def test_get_pipette_location_with_current_location_different_pipette( ) decoy.when( - labware_view.get_has_quirk( + labware_view.get_should_center_column_on_target_well( + "reservoir-id", + ) + ).then_return(False) + + decoy.when( + labware_view.get_should_center_pipette_on_target_well( "reservoir-id", - "centerMultichannelOnWells", ) ).then_return(False) @@ -171,13 +208,13 @@ def test_get_pipette_location_with_current_location_different_pipette( ) -def test_get_pipette_location_override_current_location( +def test_get_pipette_location_override_current_location_xy_center( decoy: Decoy, labware_view: LabwareView, pipette_view: PipetteView, subject: MotionView, ) -> None: - """It should calculate pipette location from a passed in deck location.""" + """It should calculate pipette location from a passed in deck location with xy override.""" current_well = CurrentWell( pipette_id="pipette-id", labware_id="reservoir-id", @@ -193,9 +230,8 @@ def test_get_pipette_location_override_current_location( ) decoy.when( - labware_view.get_has_quirk( + labware_view.get_should_center_pipette_on_target_well( "reservoir-id", - "centerMultichannelOnWells", ) ).then_return(True) @@ -210,7 +246,127 @@ def test_get_pipette_location_override_current_location( ) -def test_get_movement_waypoints_to_well( +def test_get_pipette_location_override_current_location_y_center( + decoy: Decoy, + labware_view: LabwareView, + pipette_view: PipetteView, + subject: MotionView, +) -> None: + """It should calculate pipette location from a passed in deck location with xy override.""" + current_well = CurrentWell( + pipette_id="pipette-id", + labware_id="reservoir-id", + well_name="A1", + ) + + decoy.when(pipette_view.get("pipette-id")).then_return( + LoadedPipette( + id="pipette-id", + mount=MountType.RIGHT, + pipetteName=PipetteNameType.P300_SINGLE, + ) + ) + + decoy.when( + labware_view.get_should_center_column_on_target_well( + "reservoir-id", + ) + ).then_return(True) + + result = subject.get_pipette_location( + pipette_id="pipette-id", + current_location=current_well, + ) + + assert result == PipetteLocationData( + mount=MountType.RIGHT, + critical_point=CriticalPoint.Y_CENTER, + ) + + +def test_get_movement_waypoints_to_well_for_y_center( + decoy: Decoy, + labware_view: LabwareView, + pipette_view: PipetteView, + geometry_view: GeometryView, + mock_module_view: ModuleView, + subject: MotionView, +) -> None: + """It should call get_waypoints() with the correct args to move to a well.""" + location = CurrentWell(pipette_id="123", labware_id="456", well_name="abc") + + decoy.when(pipette_view.get_current_location()).then_return(location) + + decoy.when( + labware_view.get_should_center_column_on_target_well( + "labware-id", + ) + ).then_return(True) + decoy.when( + labware_view.get_should_center_pipette_on_target_well( + "labware-id", + ) + ).then_return(False) + + decoy.when( + geometry_view.get_well_position("labware-id", "well-name", WellLocation()) + ).then_return(Point(x=4, y=5, z=6)) + + decoy.when( + move_types.get_move_type_to_well( + "pipette-id", "labware-id", "well-name", location, True + ) + ).then_return(motion_planning.MoveType.GENERAL_ARC) + decoy.when( + geometry_view.get_min_travel_z("pipette-id", "labware-id", location, 123) + ).then_return(42.0) + + decoy.when(geometry_view.get_ancestor_slot_name("labware-id")).then_return( + DeckSlotName.SLOT_2 + ) + + decoy.when( + geometry_view.get_extra_waypoints(location, DeckSlotName.SLOT_2) + ).then_return([(456, 789)]) + + waypoints = [ + motion_planning.Waypoint( + position=Point(1, 2, 3), critical_point=CriticalPoint.Y_CENTER + ), + motion_planning.Waypoint( + position=Point(4, 5, 6), critical_point=CriticalPoint.MOUNT + ), + ] + + decoy.when( + motion_planning.get_waypoints( + move_type=motion_planning.MoveType.GENERAL_ARC, + origin=Point(x=1, y=2, z=3), + origin_cp=CriticalPoint.MOUNT, + max_travel_z=1337, + min_travel_z=42, + dest=Point(x=4, y=5, z=6), + dest_cp=CriticalPoint.Y_CENTER, + xy_waypoints=[(456, 789)], + ) + ).then_return(waypoints) + + result = subject.get_movement_waypoints_to_well( + pipette_id="pipette-id", + labware_id="labware-id", + well_name="well-name", + well_location=WellLocation(), + origin=Point(x=1, y=2, z=3), + origin_cp=CriticalPoint.MOUNT, + max_travel_z=1337, + force_direct=True, + minimum_z_height=123, + ) + + assert result == waypoints + + +def test_get_movement_waypoints_to_well_for_xy_center( decoy: Decoy, labware_view: LabwareView, pipette_view: PipetteView, @@ -222,8 +378,16 @@ def test_get_movement_waypoints_to_well( location = CurrentWell(pipette_id="123", labware_id="456", well_name="abc") decoy.when(pipette_view.get_current_location()).then_return(location) + decoy.when( - labware_view.get_has_quirk("labware-id", "centerMultichannelOnWells") + labware_view.get_should_center_column_on_target_well( + "labware-id", + ) + ).then_return(False) + decoy.when( + labware_view.get_should_center_pipette_on_target_well( + "labware-id", + ) ).then_return(True) decoy.when( @@ -597,8 +761,11 @@ def test_get_touch_tip_waypoints( center_point = Point(1, 2, 3) decoy.when( - labware_view.get_has_quirk("labware-id", "centerMultichannelOnWells") + labware_view.get_should_center_pipette_on_target_well("labware-id") ).then_return(True) + decoy.when( + labware_view.get_should_center_column_on_target_well("labware-id") + ).then_return(False) decoy.when(pipette_view.get_mount("pipette-id")).then_return(MountType.LEFT) From 2d0bcfdc078f86fd01620bc3d7a4be4789831782 Mon Sep 17 00:00:00 2001 From: Jamey H Date: Fri, 15 Dec 2023 17:59:15 -0500 Subject: [PATCH 19/32] fix(app): Fix persistent cancelled state (#14228) * fix(app): add dimsissing current run back * fix(app): fix robot stuck in cancelled state * test fixes --- .../Devices/ProtocolRun/ProtocolRunHeader.tsx | 16 +++++++++++++--- .../__tests__/ProtocolRunHeader.test.tsx | 19 +++++++++++++++++-- .../RunningProtocol/ConfirmCancelRunModal.tsx | 12 ++++++++++-- .../__tests__/ConfirmCancelRunModal.test.tsx | 13 +++++++++++-- app/src/pages/OnDeviceDisplay/RunSummary.tsx | 8 +++++++- 5 files changed, 58 insertions(+), 10 deletions(-) diff --git a/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx b/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx index 8c38f917afa..50f5b5b6c2a 100644 --- a/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx +++ b/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx @@ -106,6 +106,7 @@ import { RunProgressMeter } from '../../RunProgressMeter' import { getIsFixtureMismatch } from '../../../resources/deck_configuration/utils' import { useDeckConfigurationCompatibility } from '../../../resources/deck_configuration/hooks' import { useMostRecentCompletedAnalysis } from '../../LabwarePositionCheck/useMostRecentCompletedAnalysis' +import { useMostRecentRunId } from '../../ProtocolUpload/hooks/useMostRecentRunId' import type { Run, RunError } from '@opentrons/api-client' import type { State } from '../../../redux/types' @@ -161,6 +162,7 @@ export function ProtocolRunHeader({ const { analysisErrors } = useProtocolAnalysisErrors(runId) const { data: attachedInstruments } = useInstrumentsQuery() const isRunCurrent = Boolean(useRunQuery(runId)?.data?.data?.current) + const mostRecentRunId = useMostRecentRunId() const { closeCurrentRun, isClosingCurrentRun } = useCloseCurrentRun() const { startedAt, stoppedAt, completedAt } = useRunTimestamps(runId) const [showRunFailedModal, setShowRunFailedModal] = React.useState(false) @@ -249,8 +251,9 @@ export function ProtocolRunHeader({ ...robotAnalyticsData, }, }) + closeCurrentRun() } - }, [runStatus, isRunCurrent, runId]) + }, [runStatus, isRunCurrent, runId, closeCurrentRun]) const startedAtTimestamp = startedAt != null ? formatTimestamp(startedAt) : EMPTY_TIMESTAMP @@ -372,7 +375,9 @@ export function ProtocolRunHeader({ }} /> ) : null} - {isRunCurrent && showDropTipBanner && pipettesWithTip.length !== 0 ? ( + {mostRecentRunId === runId && + showDropTipBanner && + pipettesWithTip.length !== 0 ? ( { @@ -786,6 +791,11 @@ function TerminalRunBanner(props: TerminalRunProps): JSX.Element | null { } = props const { t } = useTranslation('run_details') + const handleClick = (): void => { + handleClearClick() + setShowRunFailedModal(true) + } + if (runStatus === RUN_STATUS_FAILED || runStatus === RUN_STATUS_SUCCEEDED) { return ( <> @@ -810,7 +820,7 @@ function TerminalRunBanner(props: TerminalRunProps): JSX.Element | null { setShowRunFailedModal(true)} + onClick={handleClick} textDecoration={TYPOGRAPHY.textDecorationUnderline} > {t('view_error')} diff --git a/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx b/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx index cad5de28172..4dc85eb7dcf 100644 --- a/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx +++ b/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx @@ -22,6 +22,7 @@ import { useRunQuery, useModulesQuery, usePipettesQuery, + useDismissCurrentRunMutation, useEstopQuery, useDoorQuery, useInstrumentsQuery, @@ -91,6 +92,7 @@ import { getPipettesWithTipAttached } from '../../../DropTipWizard/getPipettesWi import { getIsFixtureMismatch } from '../../../../resources/deck_configuration/utils' import { useDeckConfigurationCompatibility } from '../../../../resources/deck_configuration/hooks' import { useMostRecentCompletedAnalysis } from '../../../LabwarePositionCheck/useMostRecentCompletedAnalysis' +import { useMostRecentRunId } from '../../../ProtocolUpload/hooks/useMostRecentRunId' import type { UseQueryResult } from 'react-query' import type { Run } from '@opentrons/api-client' @@ -139,6 +141,7 @@ jest.mock('../../../DropTipWizard/getPipettesWithTipAttached') jest.mock('../../../../resources/deck_configuration/utils') jest.mock('../../../../resources/deck_configuration/hooks') jest.mock('../../../LabwarePositionCheck/useMostRecentCompletedAnalysis') +jest.mock('../../../ProtocolUpload/hooks/useMostRecentRunId') const mockGetIsHeaterShakerAttached = getIsHeaterShakerAttached as jest.MockedFunction< typeof getIsHeaterShakerAttached @@ -189,6 +192,9 @@ const mockUsePipettesQuery = usePipettesQuery as jest.MockedFunction< const mockConfirmCancelModal = ConfirmCancelModal as jest.MockedFunction< typeof ConfirmCancelModal > +const mockUseDismissCurrentRunMutation = useDismissCurrentRunMutation as jest.MockedFunction< + typeof useDismissCurrentRunMutation +> const mockHeaterShakerIsRunningModal = HeaterShakerIsRunningModal as jest.MockedFunction< typeof HeaterShakerIsRunningModal > @@ -242,6 +248,9 @@ const mockUseDeckConfigurationCompatibility = useDeckConfigurationCompatibility const mockUseMostRecentCompletedAnalysis = useMostRecentCompletedAnalysis as jest.MockedFunction< typeof useMostRecentCompletedAnalysis > +const mockUseMostRecentRunId = useMostRecentRunId as jest.MockedFunction< + typeof useMostRecentRunId +> const ROBOT_NAME = 'otie' const RUN_ID = '95e67900-bc9f-4fbf-92c6-cc4d7226a51b' @@ -398,6 +407,11 @@ describe('ProtocolRunHeader', () => { when(mockUseTrackProtocolRunEvent).calledWith(RUN_ID).mockReturnValue({ trackProtocolRunEvent: mockTrackProtocolRunEvent, }) + when(mockUseDismissCurrentRunMutation) + .calledWith() + .mockReturnValue({ + dismissCurrentRun: jest.fn(), + } as any) when(mockUseUnmatchedModulesForProtocol) .calledWith(ROBOT_NAME, RUN_ID) .mockReturnValue({ missingModuleIds: [], remainingAttachedModules: [] }) @@ -429,6 +443,7 @@ describe('ProtocolRunHeader', () => { } as any) mockUseDeckConfigurationCompatibility.mockReturnValue([]) when(mockGetIsFixtureMismatch).mockReturnValue(false) + when(mockUseMostRecentRunId).mockReturnValue(RUN_ID) }) afterEach(() => { @@ -514,7 +529,7 @@ describe('ProtocolRunHeader', () => { data: { data: { ...mockIdleUnstartedRun, current: true } }, } as UseQueryResult) render() - expect(mockCloseCurrentRun).not.toBeCalled() + expect(mockCloseCurrentRun).toBeCalled() expect(mockTrackProtocolRunEvent).toBeCalled() expect(mockTrackProtocolRunEvent).toBeCalledWith({ name: ANALYTICS_PROTOCOL_RUN_FINISH, @@ -822,7 +837,7 @@ describe('ProtocolRunHeader', () => { const [{ getByText }] = render() getByText('View error').click() - expect(mockCloseCurrentRun).not.toHaveBeenCalled() + expect(mockCloseCurrentRun).toBeCalled() getByText('mock RunFailedModal') }) diff --git a/app/src/organisms/OnDeviceDisplay/RunningProtocol/ConfirmCancelRunModal.tsx b/app/src/organisms/OnDeviceDisplay/RunningProtocol/ConfirmCancelRunModal.tsx index 771417f33c3..1b6996ba885 100644 --- a/app/src/organisms/OnDeviceDisplay/RunningProtocol/ConfirmCancelRunModal.tsx +++ b/app/src/organisms/OnDeviceDisplay/RunningProtocol/ConfirmCancelRunModal.tsx @@ -10,7 +10,10 @@ import { Flex, SPACING, } from '@opentrons/components' -import { useStopRunMutation } from '@opentrons/react-api-client' +import { + useStopRunMutation, + useDismissCurrentRunMutation, +} from '@opentrons/react-api-client' import { StyledText } from '../../../atoms/text' import { SmallButton } from '../../../atoms/buttons' @@ -37,6 +40,10 @@ export function ConfirmCancelRunModal({ }: ConfirmCancelRunModalProps): JSX.Element { const { t } = useTranslation(['run_details', 'shared']) const { stopRun } = useStopRunMutation() + const { + dismissCurrentRun, + isLoading: isDismissing, + } = useDismissCurrentRunMutation() const runStatus = useRunStatus(runId) const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId) const history = useHistory() @@ -61,6 +68,7 @@ export function ConfirmCancelRunModal({ React.useEffect(() => { if (runStatus === RUN_STATUS_STOPPED) { trackProtocolRunEvent({ name: ANALYTICS_PROTOCOL_RUN_CANCEL }) + dismissCurrentRun(runId) if (!isActiveRun) { if (protocolId != null) { history.push(`/protocols/${protocolId}`) @@ -71,7 +79,7 @@ export function ConfirmCancelRunModal({ } }, [runStatus]) - return isCanceling ? ( + return isCanceling || isDismissing ? ( ) : ( { getByText('Cancel run') }) + it('shoudler render the canceling run modal when run is dismissing', () => { + mockUseDismissCurrentRunMutation.mockReturnValue({ + dismissCurrentRun: mockDismissCurrentRun, + isLoading: true, + } as any) + const [{ getByText }] = render(props) + getByText('mock CancelingRunModal') + }) + it('when tapping go back, the mock function is called', () => { const [{ getByText }] = render(props) const button = getByText('Go back') @@ -138,7 +147,7 @@ describe('ConfirmCancelRunModal', () => { .mockReturnValue(RUN_STATUS_STOPPED) render(props) - expect(mockDismissCurrentRun).not.toHaveBeenCalled() + expect(mockDismissCurrentRun).toHaveBeenCalled() expect(mockTrackProtocolRunEvent).toHaveBeenCalled() }) @@ -152,7 +161,7 @@ describe('ConfirmCancelRunModal', () => { .mockReturnValue(RUN_STATUS_STOPPED) render(props) - expect(mockDismissCurrentRun).not.toHaveBeenCalled() + expect(mockDismissCurrentRun).toHaveBeenCalled() expect(mockTrackProtocolRunEvent).toHaveBeenCalled() expect(mockPush).toHaveBeenCalledWith('/protocols') }) diff --git a/app/src/pages/OnDeviceDisplay/RunSummary.tsx b/app/src/pages/OnDeviceDisplay/RunSummary.tsx index 58fda92a385..6b7a06ac62d 100644 --- a/app/src/pages/OnDeviceDisplay/RunSummary.tsx +++ b/app/src/pages/OnDeviceDisplay/RunSummary.tsx @@ -63,6 +63,7 @@ import { formatTimeWithUtcLabel } from '../../resources/runs/utils' import { handleTipsAttachedModal } from '../../organisms/DropTipWizard/TipsAttachedModal' import { getPipettesWithTipAttached } from '../../organisms/DropTipWizard/getPipettesWithTipAttached' import { getPipetteModelSpecs, FLEX_ROBOT_TYPE } from '@opentrons/shared-data' +import { useMostRecentRunId } from '../../organisms/ProtocolUpload/hooks/useMostRecentRunId' import type { OnDeviceRouteParams } from '../../App/types' import type { PipetteModelSpecs } from '@opentrons/shared-data' @@ -79,6 +80,7 @@ export function RunSummary(): JSX.Element { const host = useHost() const { data: runRecord } = useRunQuery(runId, { staleTime: Infinity }) const isRunCurrent = Boolean(runRecord?.data?.current) + const mostRecentRunId = useMostRecentRunId() const { data: attachedInstruments } = useInstrumentsQuery() const runStatus = runRecord?.data.status ?? null const didRunSucceed = runStatus === RUN_STATUS_SUCCEEDED @@ -130,7 +132,11 @@ export function RunSummary(): JSX.Element { const handleReturnToDash = (): void => { const { mount, specs } = pipettesWithTip[0] || {} - if (isRunCurrent && pipettesWithTip.length !== 0 && specs != null) { + if ( + mostRecentRunId === runId && + pipettesWithTip.length !== 0 && + specs != null + ) { handleTipsAttachedModal( mount, specs, From 5fc0e2965c317adcd448554703db8a5915f48abf Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Mon, 18 Dec 2023 10:30:57 -0500 Subject: [PATCH 20/32] test(api): Add integration tests for load conflicts with the fixed trash (#14151) --- .../protocol_api_integration/test_trashes.py | 70 ++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/api/tests/opentrons/protocol_api_integration/test_trashes.py b/api/tests/opentrons/protocol_api_integration/test_trashes.py index 1ad9a945c9a..c9b31c08167 100644 --- a/api/tests/opentrons/protocol_api_integration/test_trashes.py +++ b/api/tests/opentrons/protocol_api_integration/test_trashes.py @@ -3,7 +3,8 @@ from opentrons import protocol_api, simulate -from typing import Optional, Type +import contextlib +from typing import ContextManager, Optional, Type from typing_extensions import Literal import pytest @@ -96,3 +97,70 @@ def test_trash_search() -> None: # You should be able to override instrument.trash_container explicitly. instrument.trash_container = loaded_second assert instrument.trash_container is loaded_second + + +@pytest.mark.parametrize( + ("version", "robot_type", "expect_load_to_succeed"), + [ + pytest.param( + "2.13", + "OT-2", + False, + # This xfail (the system does let you load a labware onto slot 12, and does not raise) + # is surprising to me. It may be be a bug in old PAPI versions. + marks=pytest.mark.xfail(strict=True, raises=pytest.fail.Exception), + ), + ("2.14", "OT-2", False), + ("2.15", "OT-2", False), + pytest.param( + "2.15", + "Flex", + False, + marks=pytest.mark.ot3_only, # Simulating a Flex protocol requires a Flex hardware API. + ), + pytest.param( + "2.16", + "OT-2", + False, + # This should ideally raise, matching OT-2 behavior on prior Protocol API versions. + # It currently does not because Protocol API v2.15's trashes are implemented as + # addressable areas, not labware--and they're only brought into existence + # *on first use,* not at the beginning of a protocol. + # + # The good news is that even though the conflicting load will not raise like we want, + # something in the protocol will eventually raise, e.g. when a pipette goes to drop a + # tip in the fixed trash and finds that a fixed trash can't exist there because there's + # a labware. + marks=pytest.mark.xfail(strict=True, raises=pytest.fail.Exception), + ), + pytest.param( + "2.16", + "Flex", + True, + marks=pytest.mark.ot3_only, # Simulating a Flex protocol requires a Flex hardware API. + ), + ], +) +def test_fixed_trash_load_conflicts( + robot_type: Literal["Flex", "OT-2"], + version: str, + expect_load_to_succeed: bool, +) -> None: + """Test loading something onto the location historically used for the fixed trash. + + In configurations where there is a fixed trash, this should be disallowed. + In configurations without a fixed trash, this should be allowed. + """ + protocol = simulate.get_protocol_api(version=version, robot_type=robot_type) + + if expect_load_to_succeed: + expected_error: ContextManager[object] = contextlib.nullcontext() + else: + expected_error = pytest.raises( + Exception, + # Exact message doesn't matter, as long as it's definitely a labware load conflict. + match="LocationIsOccupiedError", + ) + + with expected_error: + protocol.load_labware("opentrons_96_wellplate_200ul_pcr_full_skirt", 12) From 5a939eafc8f7c0fdcace8aa3109ab5f65bac35b6 Mon Sep 17 00:00:00 2001 From: Derek Maggio Date: Mon, 18 Dec 2023 09:22:07 -0800 Subject: [PATCH 21/32] fix(api): allow volume 0 commands in engine (#14211) A previous PR (12a630b / #13989 ) changed the python protocol api in version 2.16 to allow commanding 0ul liquid handling commands like aspirate, mix, and dispense. This is useful in programmatic protocols that read out volumes to handle; they can now handle 0 volume properly. In 2.15 and previous, specifying 0 would lead to those commands doing the most volume they could (i.e. aspirate the full tip volume, dispense whatever's currently in the pipette, mix at full volume) and this likely was an unintentional side effect because of python truthiness. However, that change only touched the python protocol API; that API would emit commands to the engine that specified 0 volume, and those were not allowed: the pydantic models for the commands and responses required strictly greater than 0 volume. This PR - changes the pydantic models and updates the schema to allow 0ul commands - adds a python protocol to be an integration test - adds unit tests for the python protocol api aspirate and dispense commands --------- Co-authored-by: Seth Foster Co-authored-by: Max Marrone --- .../commands/pipetting_common.py | 6 +- .../protocol_api/test_instrument_context.py | 137 ++++++++++++++++++ ...C_HS_TM_2_16_aspirateDispenseMix0Volume.py | 71 +++++++++ shared-data/command/schemas/8.json | 20 +-- 4 files changed, 221 insertions(+), 13 deletions(-) create mode 100644 app-testing/files/protocols/py/OT2_P300M_P20S_TC_HS_TM_2_16_aspirateDispenseMix0Volume.py diff --git a/api/src/opentrons/protocol_engine/commands/pipetting_common.py b/api/src/opentrons/protocol_engine/commands/pipetting_common.py index a2dc1c8e5cd..77d4769bbd3 100644 --- a/api/src/opentrons/protocol_engine/commands/pipetting_common.py +++ b/api/src/opentrons/protocol_engine/commands/pipetting_common.py @@ -19,9 +19,9 @@ class VolumeMixin(BaseModel): volume: float = Field( ..., - description="Amount of liquid in uL. Must be greater than 0 and less " + description="Amount of liquid in uL. Must be at least 0 and no greater " "than a pipette-specific maximum volume.", - gt=0, + ge=0, ) @@ -88,7 +88,7 @@ class BaseLiquidHandlingResult(BaseModel): volume: float = Field( ..., description="Amount of liquid in uL handled in the operation.", - gt=0, + ge=0, ) diff --git a/api/tests/opentrons/protocol_api/test_instrument_context.py b/api/tests/opentrons/protocol_api/test_instrument_context.py index 5b9b2e422f9..328aed5b01f 100644 --- a/api/tests/opentrons/protocol_api/test_instrument_context.py +++ b/api/tests/opentrons/protocol_api/test_instrument_context.py @@ -990,3 +990,140 @@ def test_configure_nozzle_layout( """The correct model is passed to the engine client.""" with exception: subject.configure_nozzle_layout(style, primary_nozzle, front_right_nozzle) + + +@pytest.mark.parametrize("api_version", [APIVersion(2, 15)]) +def test_dispense_0_volume_means_dispense_everything( + decoy: Decoy, + mock_instrument_core: InstrumentCore, + subject: InstrumentContext, + mock_protocol_core: ProtocolCore, +) -> None: + """It should dispense all liquid to a well.""" + input_location = Location(point=Point(2, 2, 2), labware=None) + decoy.when( + mock_validation.validate_location(location=input_location, last_location=None) + ).then_return(mock_validation.PointTarget(location=input_location, in_place=False)) + decoy.when(mock_instrument_core.get_current_volume()).then_return(100) + decoy.when(mock_instrument_core.get_dispense_flow_rate(1.23)).then_return(5.67) + subject.dispense(volume=0, location=input_location, rate=1.23, push_out=None) + + decoy.verify( + mock_instrument_core.dispense( + location=input_location, + well_core=None, + in_place=False, + volume=100, + rate=1.23, + flow_rate=5.67, + push_out=None, + ), + times=1, + ) + + +@pytest.mark.parametrize("api_version", [APIVersion(2, 16)]) +def test_dispense_0_volume_means_dispense_nothing( + decoy: Decoy, + mock_instrument_core: InstrumentCore, + subject: InstrumentContext, + mock_protocol_core: ProtocolCore, +) -> None: + """It should dispense no liquid to a well.""" + input_location = Location(point=Point(2, 2, 2), labware=None) + decoy.when( + mock_validation.validate_location(location=input_location, last_location=None) + ).then_return(mock_validation.PointTarget(location=input_location, in_place=False)) + decoy.when(mock_instrument_core.get_dispense_flow_rate(1.23)).then_return(5.67) + subject.dispense(volume=0, location=input_location, rate=1.23, push_out=None) + + decoy.verify( + mock_instrument_core.dispense( + location=input_location, + well_core=None, + in_place=False, + volume=0, + rate=1.23, + flow_rate=5.67, + push_out=None, + ), + times=1, + ) + + +@pytest.mark.parametrize("api_version", [APIVersion(2, 15)]) +def test_aspirate_0_volume_means_aspirate_everything( + decoy: Decoy, + mock_instrument_core: InstrumentCore, + subject: InstrumentContext, + mock_protocol_core: ProtocolCore, +) -> None: + """It should aspirate to a well.""" + mock_well = decoy.mock(cls=Well) + input_location = Location(point=Point(2, 2, 2), labware=mock_well) + last_location = Location(point=Point(9, 9, 9), labware=None) + decoy.when(mock_instrument_core.get_mount()).then_return(Mount.RIGHT) + + decoy.when(mock_protocol_core.get_last_location(Mount.RIGHT)).then_return( + last_location + ) + + decoy.when( + mock_validation.validate_location( + location=input_location, last_location=last_location + ) + ).then_return(WellTarget(well=mock_well, location=input_location, in_place=False)) + decoy.when(mock_instrument_core.get_aspirate_flow_rate(1.23)).then_return(5.67) + decoy.when(mock_instrument_core.get_available_volume()).then_return(200) + subject.aspirate(volume=0, location=input_location, rate=1.23) + + decoy.verify( + mock_instrument_core.aspirate( + location=input_location, + well_core=mock_well._core, + in_place=False, + volume=200, + rate=1.23, + flow_rate=5.67, + ), + times=1, + ) + + +@pytest.mark.parametrize("api_version", [APIVersion(2, 16)]) +def test_aspirate_0_volume_means_aspirate_nothing( + decoy: Decoy, + mock_instrument_core: InstrumentCore, + subject: InstrumentContext, + mock_protocol_core: ProtocolCore, +) -> None: + """It should aspirate to a well.""" + mock_well = decoy.mock(cls=Well) + input_location = Location(point=Point(2, 2, 2), labware=mock_well) + last_location = Location(point=Point(9, 9, 9), labware=None) + decoy.when(mock_instrument_core.get_mount()).then_return(Mount.RIGHT) + + decoy.when(mock_protocol_core.get_last_location(Mount.RIGHT)).then_return( + last_location + ) + + decoy.when( + mock_validation.validate_location( + location=input_location, last_location=last_location + ) + ).then_return(WellTarget(well=mock_well, location=input_location, in_place=False)) + decoy.when(mock_instrument_core.get_aspirate_flow_rate(1.23)).then_return(5.67) + + subject.aspirate(volume=0, location=input_location, rate=1.23) + + decoy.verify( + mock_instrument_core.aspirate( + location=input_location, + well_core=mock_well._core, + in_place=False, + volume=0, + rate=1.23, + flow_rate=5.67, + ), + times=1, + ) diff --git a/app-testing/files/protocols/py/OT2_P300M_P20S_TC_HS_TM_2_16_aspirateDispenseMix0Volume.py b/app-testing/files/protocols/py/OT2_P300M_P20S_TC_HS_TM_2_16_aspirateDispenseMix0Volume.py new file mode 100644 index 00000000000..edf43366e1a --- /dev/null +++ b/app-testing/files/protocols/py/OT2_P300M_P20S_TC_HS_TM_2_16_aspirateDispenseMix0Volume.py @@ -0,0 +1,71 @@ +"""Smoke Test v3.0 """ +from opentrons import protocol_api + +metadata = { + "protocolName": "API 2.16 Aspirate Dispense Mix 0 Volume", + "author": "Opentrons Engineering ", + "source": "Software Testing Team", +} + +requirements = {"robotType": "OT-2", "apiLevel": "2.16"} + + +def run(ctx: protocol_api.ProtocolContext) -> None: + """This method is run by the protocol engine.""" + + ctx.set_rail_lights(True) + + # deck positions + tips_300ul_position = "5" + tips_20ul_position = "4" + dye_source_position = "3" + logo_position = "2" + + # 300ul tips + tips_300ul = [ + ctx.load_labware( + load_name="opentrons_96_tiprack_300ul", + location=tips_300ul_position, + label="300ul tips", + ) + ] + + # 20ul tips + tips_20ul = [ + ctx.load_labware( + load_name="opentrons_96_tiprack_20ul", + location=tips_20ul_position, + label="20ul tips", + ) + ] + + # pipettes + ctx.load_instrument(instrument_name="p300_multi_gen2", mount="left", tip_racks=tips_300ul) + + pipette_right = ctx.load_instrument(instrument_name="p20_single_gen2", mount="right", tip_racks=tips_20ul) + + dye_container = ctx.load_labware( + load_name="nest_12_reservoir_15ml", + location=dye_source_position, + label="dye container", + ) + + # >= 2.14 define_liquid and load_liquid + water = ctx.define_liquid( + name="water", description="H₂O", display_color="#42AB2D" + ) # subscript 2 https://www.compart.com/en/unicode/U+2082 + + dye_container.wells_by_name()["A1"].load_liquid(liquid=water, volume=20) + + pipette_right.pick_up_tip() + + # >= 2.15: Aspirate everything, then dispense everything + # < 2.15: Aspirate nothing, then dispense everything(Which in this case means nothing) + # pipette_right.aspirate(volume=0, location=dye_container.wells_by_name()["A1"]) + # pipette_right.dispense(location=dye_container.wells_by_name()["A1"]) + + # >= 2.15: Aspirate everything, dispense everything, mix everything + # < 2.15: Aspirate everything, dispense nothing, mix nothing + pipette_right.aspirate(volume=20, location=dye_container.wells_by_name()["A1"]) + pipette_right.dispense(volume=0, location=dye_container.wells_by_name()["A1"]) + pipette_right.mix(volume=0, location=dye_container.wells_by_name()["A1"]) diff --git a/shared-data/command/schemas/8.json b/shared-data/command/schemas/8.json index 1cff4c6ef11..c95c466db5e 100644 --- a/shared-data/command/schemas/8.json +++ b/shared-data/command/schemas/8.json @@ -259,8 +259,8 @@ }, "volume": { "title": "Volume", - "description": "Amount of liquid in uL. Must be greater than 0 and less than a pipette-specific maximum volume.", - "exclusiveMinimum": 0, + "description": "Amount of liquid in uL. Must be at least 0 and no greater than a pipette-specific maximum volume.", + "minimum": 0, "type": "number" }, "pipetteId": { @@ -320,8 +320,8 @@ }, "volume": { "title": "Volume", - "description": "Amount of liquid in uL. Must be greater than 0 and less than a pipette-specific maximum volume.", - "exclusiveMinimum": 0, + "description": "Amount of liquid in uL. Must be at least 0 and no greater than a pipette-specific maximum volume.", + "minimum": 0, "type": "number" }, "pipetteId": { @@ -412,8 +412,8 @@ "properties": { "volume": { "title": "Volume", - "description": "Amount of liquid in uL. Must be greater than 0 and less than a pipette-specific maximum volume.", - "exclusiveMinimum": 0, + "description": "Amount of liquid in uL. Must be at least 0 and no greater than a pipette-specific maximum volume.", + "minimum": 0, "type": "number" }, "pipetteId": { @@ -684,8 +684,8 @@ }, "volume": { "title": "Volume", - "description": "Amount of liquid in uL. Must be greater than 0 and less than a pipette-specific maximum volume.", - "exclusiveMinimum": 0, + "description": "Amount of liquid in uL. Must be at least 0 and no greater than a pipette-specific maximum volume.", + "minimum": 0, "type": "number" }, "pipetteId": { @@ -744,8 +744,8 @@ }, "volume": { "title": "Volume", - "description": "Amount of liquid in uL. Must be greater than 0 and less than a pipette-specific maximum volume.", - "exclusiveMinimum": 0, + "description": "Amount of liquid in uL. Must be at least 0 and no greater than a pipette-specific maximum volume.", + "minimum": 0, "type": "number" }, "pipetteId": { From d2e9f57f87b29a35b5b9703108255d1cff74e9ff Mon Sep 17 00:00:00 2001 From: Alise Au <20424172+ahiuchingau@users.noreply.github.com> Date: Mon, 18 Dec 2023 12:41:37 -0500 Subject: [PATCH 22/32] reverting the grip force back to 15N (#14221) --- .../2/armadillo_96_wellplate_200ul_pcr_full_skirt/2.json | 2 +- .../2/opentrons_96_wellplate_200ul_pcr_full_skirt/2.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/shared-data/labware/definitions/2/armadillo_96_wellplate_200ul_pcr_full_skirt/2.json b/shared-data/labware/definitions/2/armadillo_96_wellplate_200ul_pcr_full_skirt/2.json index dec71b2b4e1..f0704d43cca 100644 --- a/shared-data/labware/definitions/2/armadillo_96_wellplate_200ul_pcr_full_skirt/2.json +++ b/shared-data/labware/definitions/2/armadillo_96_wellplate_200ul_pcr_full_skirt/2.json @@ -55,7 +55,7 @@ "z": 10.7 } }, - "gripForce": 9, + "gripForce": 15, "gripHeightFromLabwareBottom": 10, "ordering": [ ["A1", "B1", "C1", "D1", "E1", "F1", "G1", "H1"], diff --git a/shared-data/labware/definitions/2/opentrons_96_wellplate_200ul_pcr_full_skirt/2.json b/shared-data/labware/definitions/2/opentrons_96_wellplate_200ul_pcr_full_skirt/2.json index 4e8314698aa..fb6420bac45 100644 --- a/shared-data/labware/definitions/2/opentrons_96_wellplate_200ul_pcr_full_skirt/2.json +++ b/shared-data/labware/definitions/2/opentrons_96_wellplate_200ul_pcr_full_skirt/2.json @@ -55,7 +55,7 @@ "z": 10.7 } }, - "gripForce": 9, + "gripForce": 15, "gripHeightFromLabwareBottom": 10, "ordering": [ ["A1", "B1", "C1", "D1", "E1", "F1", "G1", "H1"], From 70819cb0673032ac8e7aec016a4ec80db9b79234 Mon Sep 17 00:00:00 2001 From: Ed Cormany Date: Mon, 18 Dec 2023 14:06:27 -0500 Subject: [PATCH 23/32] docs(api): Versioning page updates for Python API 2.16 (#14073) --------- Co-authored-by: Max Marrone --- api/docs/v2/conf.py | 2 +- api/docs/v2/versioning.rst | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/api/docs/v2/conf.py b/api/docs/v2/conf.py index cb896e72d89..e3c68989a46 100644 --- a/api/docs/v2/conf.py +++ b/api/docs/v2/conf.py @@ -99,7 +99,7 @@ # use rst_prolog to hold the subsitution # update the apiLevel value whenever a new minor version is released rst_prolog = f""" -.. |apiLevel| replace:: 2.15 +.. |apiLevel| replace:: 2.16 .. |release| replace:: {release} """ diff --git a/api/docs/v2/versioning.rst b/api/docs/v2/versioning.rst index f635a84812f..1c1fc187514 100644 --- a/api/docs/v2/versioning.rst +++ b/api/docs/v2/versioning.rst @@ -66,9 +66,9 @@ The maximum supported API version for your robot is listed in the Opentrons App If you upload a protocol that specifies a higher API level than the maximum supported, your robot won't be able to analyze or run your protocol. You can increase the maximum supported version by updating your robot software and Opentrons App. -Opentrons robots running the latest software (7.0.0) support the following version ranges: +Opentrons robots running the latest software (7.1.0) support the following version ranges: - * **Flex:** version 2.15. + * **Flex:** version 2.15–|apiLevel|. * **OT-2:** versions 2.0–|apiLevel|. @@ -82,6 +82,8 @@ This table lists the correspondence between Protocol API versions and robot soft +-------------+------------------------------+ | API Version | Introduced in Robot Software | +=============+==============================+ +| 2.16 | 7.1.0 | ++-------------+------------------------------+ | 2.15 | 7.0.0 | +-------------+------------------------------+ | 2.14 | 6.3.0 | @@ -122,6 +124,32 @@ This table lists the correspondence between Protocol API versions and robot soft Changes in API Versions ======================= +Version 2.16 +------------ + +This version introduces new features for Flex and adds and improves methods for aspirating and dispensing. Note that when updating Flex protocols to version 2.16, you *must* load a trash container before dropping tips. + +- New features + + - Use :py:meth:`.configure_nozzle_layout` to pick up a single column of tips with the 96-channel pipette. See :ref:`Partial Tip Pickup `. + - Specify the trash containers attached to your Flex with :py:meth:`.load_waste_chute` and :py:meth:`.load_trash_bin`. + - Dispense, blow out, drop tips, and dispose labware in the waste chute. Disposing labware requires the gripper and calling :py:meth:`.move_labware` with ``use_gripper=True``. + - Perform actions in staging area slots by referencing slots A4 through D4. See :ref:`deck-slots`. + - Explicitly command a pipette to :py:meth:`.prepare_to_aspirate`. The API usually prepares pipettes to aspirate automatically, but this is useful for certain applications, like pre-wetting routines. + +- Improved features + + - :py:meth:`.aspirate`, :py:meth:`.dispense`, and :py:meth:`.mix` will not move any liquid when called with ``volume=0``. + +- Other changes + + - :py:obj:`.ProtocolContext.fixed_trash` and :py:obj:`.InstrumentContext.trash_container` now return :py:class:`.TrashBin` objects instead of :py:class:`.Labware` objects. + - Flex will no longer automatically drop tips in the trash at the end of a protocol. You can add a :py:meth:`.drop_tip()` command to your protocol or use the Opentrons App to drop the tips. + +- Known issues + + - It's possible to load a Thermocycler and then load another item in slot A1. Don't do this, as it could lead to unexpected pipetting behavior and crashes. + Version 2.15 ------------ From 87b299390530fb5b134c569508b9c4c9db6bf882 Mon Sep 17 00:00:00 2001 From: Jamey H Date: Mon, 18 Dec 2023 14:47:57 -0500 Subject: [PATCH 24/32] fix(app): Don't render ProtocolRunHeader TerminalBanner on CurrentRun (#14233) Closes RQA-2013 * fix(app): fix viewing error modal dismissing error and drop tip banners * refactor(app): refactor useMostRunRecentId to be safer --- .../Devices/ProtocolRun/ProtocolRunHeader.tsx | 4 ++-- .../__tests__/ProtocolRunHeader.test.tsx | 22 ------------------- .../__tests__/useMostRecentRunId.test.tsx | 9 ++++++++ .../hooks/useMostRecentRunId.ts | 5 +++-- 4 files changed, 14 insertions(+), 26 deletions(-) diff --git a/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx b/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx index 50f5b5b6c2a..ab7042fd935 100644 --- a/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx +++ b/app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx @@ -364,7 +364,7 @@ export function ProtocolRunHeader({ CANCELLABLE_STATUSES.includes(runStatus) ? ( {t('shared:close_robot_door')} ) : null} - {isRunCurrent ? ( + {mostRecentRunId === runId ? ( { expect(queryByText('Tips may be attached.')).not.toBeInTheDocument() }) }) - - it('does not show the drop tip banner when the run is not over', async () => { - when(mockUseRunQuery) - .calledWith(RUN_ID) - .mockReturnValue({ - data: { - data: { - ...mockIdleUnstartedRun, - current: false, - status: RUN_STATUS_SUCCEEDED, - }, - }, - } as UseQueryResult) - when(mockUseRunStatus) - .calledWith(RUN_ID) - .mockReturnValue(RUN_STATUS_SUCCEEDED) - - const [{ queryByText }] = render() - await waitFor(() => { - expect(queryByText('Tips may be attached.')).not.toBeInTheDocument() - }) - }) }) diff --git a/app/src/organisms/ProtocolUpload/hooks/__tests__/useMostRecentRunId.test.tsx b/app/src/organisms/ProtocolUpload/hooks/__tests__/useMostRecentRunId.test.tsx index eb0d55f639c..94c4bb30e82 100644 --- a/app/src/organisms/ProtocolUpload/hooks/__tests__/useMostRecentRunId.test.tsx +++ b/app/src/organisms/ProtocolUpload/hooks/__tests__/useMostRecentRunId.test.tsx @@ -31,6 +31,15 @@ describe('useMostRecentRunId hook', () => { const { result } = renderHook(useMostRecentRunId) + expect(result.current).toBeNull() + }) + it('should return null if no run data exists', async () => { + when(mockUseAllRunsQuery) + .calledWith() + .mockReturnValue({ data: { data: null } } as any) + + const { result } = renderHook(useMostRecentRunId) + expect(result.current).toBeNull() }) }) diff --git a/app/src/organisms/ProtocolUpload/hooks/useMostRecentRunId.ts b/app/src/organisms/ProtocolUpload/hooks/useMostRecentRunId.ts index 759a5aa2e06..f3f29c10d7a 100644 --- a/app/src/organisms/ProtocolUpload/hooks/useMostRecentRunId.ts +++ b/app/src/organisms/ProtocolUpload/hooks/useMostRecentRunId.ts @@ -1,8 +1,9 @@ import { useAllRunsQuery } from '@opentrons/react-api-client' +import last from 'lodash/last' export function useMostRecentRunId(): string | null { const { data: allRuns } = useAllRunsQuery() - return allRuns != null && allRuns.data.length > 0 - ? allRuns.data[allRuns.data.length - 1].id + return allRuns != null && allRuns.data?.length > 0 + ? last(allRuns.data)?.id ?? null : null } From 0716cad72fb6e3573c7c13534768ff4c4a8ce10f Mon Sep 17 00:00:00 2001 From: Jeremy Leon Date: Mon, 18 Dec 2023 14:57:52 -0500 Subject: [PATCH 25/32] fix(shared-data): add slot transforms for A3 for heater-shaker and temp module v2 (#14235) --- .../module/definitions/3/heaterShakerModuleV1.json | 8 ++++++++ shared-data/module/definitions/3/temperatureModuleV2.json | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/shared-data/module/definitions/3/heaterShakerModuleV1.json b/shared-data/module/definitions/3/heaterShakerModuleV1.json index 30164bd8607..fac8fa81e8d 100644 --- a/shared-data/module/definitions/3/heaterShakerModuleV1.json +++ b/shared-data/module/definitions/3/heaterShakerModuleV1.json @@ -158,6 +158,14 @@ [0, 0, 1, -49.325], [0, 0, 0, 1] ] + }, + "A3": { + "labwareOffset": [ + [1, 0, 0, 0.125], + [0, 1, 0, -1.125], + [0, 0, 1, -49.325], + [0, 0, 0, 1] + ] } } }, diff --git a/shared-data/module/definitions/3/temperatureModuleV2.json b/shared-data/module/definitions/3/temperatureModuleV2.json index 7e67659eba7..07bdfe03c47 100644 --- a/shared-data/module/definitions/3/temperatureModuleV2.json +++ b/shared-data/module/definitions/3/temperatureModuleV2.json @@ -156,6 +156,14 @@ [0, 0, 1, -71.09], [0, 0, 0, 1] ] + }, + "A3": { + "labwareOffset": [ + [1, 0, 0, 1.45], + [0, 1, 0, 0.15], + [0, 0, 1, -71.09], + [0, 0, 0, 1] + ] } } }, From 24310ce45103721c42cf277e6b40067383d0fe09 Mon Sep 17 00:00:00 2001 From: koji Date: Mon, 18 Dec 2023 16:02:11 -0500 Subject: [PATCH 26/32] fix(app): add new text to updatebanner for module card (#14237) * fix(app): add new text to updatebanner for module card --- .../assets/localization/en/device_details.json | 1 + .../__tests__/UpdateBanner.test.tsx | 9 +++++++++ app/src/molecules/UpdateBanner/index.tsx | 9 ++++++++- .../Devices/InstrumentsAndModules.tsx | 4 ++++ .../ProtocolRun/ProtocolRunModuleControls.tsx | 18 ++++++++++++++++-- .../ModuleCard/__tests__/ModuleCard.test.tsx | 1 + app/src/organisms/ModuleCard/index.tsx | 8 +++++++- 7 files changed, 46 insertions(+), 4 deletions(-) diff --git a/app/src/assets/localization/en/device_details.json b/app/src/assets/localization/en/device_details.json index 71df492e080..31a9b06b78a 100644 --- a/app/src/assets/localization/en/device_details.json +++ b/app/src/assets/localization/en/device_details.json @@ -88,6 +88,7 @@ "missing_module": "missing {{num}} module", "module_actions_unavailable": "Module actions unavailable while protocol is running", "module_calibration_required_no_pipette_attached": "Module calibration required. Attach a pipette before running module calibration.", + "module_calibration_required_no_pipette_calibrated": "Module calibration required. Calibrate pipette before running module calibration. ", "module_calibration_required_update_pipette_FW": "Update pipette firmware before proceeding with required module calibration.", "module_calibration_required": "Module calibration required.", "module_controls": "Module Controls", diff --git a/app/src/molecules/UpdateBanner/__tests__/UpdateBanner.test.tsx b/app/src/molecules/UpdateBanner/__tests__/UpdateBanner.test.tsx index c04943ca650..9cd38ce7eca 100644 --- a/app/src/molecules/UpdateBanner/__tests__/UpdateBanner.test.tsx +++ b/app/src/molecules/UpdateBanner/__tests__/UpdateBanner.test.tsx @@ -85,6 +85,14 @@ describe('Module Update Banner', () => { const { queryByText } = render(props) expect(queryByText('Calibrate now')).not.toBeInTheDocument() }) + it('should not render a calibrate link if pipette calibration is required', () => { + props = { + ...props, + calibratePipetteRequired: true, + } + const { queryByText } = render(props) + expect(queryByText('Calibrate now')).not.toBeInTheDocument() + }) it('should not render a calibrate link if pipette firmware update is required', () => { props = { ...props, @@ -98,6 +106,7 @@ describe('Module Update Banner', () => { ...props, updateType: 'firmware', attachPipetteRequired: true, + calibratePipetteRequired: true, updatePipetteFWRequired: true, } const { queryByText } = render(props) diff --git a/app/src/molecules/UpdateBanner/index.tsx b/app/src/molecules/UpdateBanner/index.tsx index 14e3b1485de..23455d46c4e 100644 --- a/app/src/molecules/UpdateBanner/index.tsx +++ b/app/src/molecules/UpdateBanner/index.tsx @@ -23,6 +23,7 @@ interface UpdateBannerProps { serialNumber: string isTooHot?: boolean attachPipetteRequired?: boolean + calibratePipetteRequired?: boolean updatePipetteFWRequired?: boolean } @@ -33,6 +34,7 @@ export const UpdateBanner = ({ setShowBanner, handleUpdateClick, attachPipetteRequired, + calibratePipetteRequired, updatePipetteFWRequired, isTooHot, }: UpdateBannerProps): JSX.Element | null => { @@ -48,11 +50,16 @@ export const UpdateBanner = ({ closeButtonRendered = false if (attachPipetteRequired) bannerMessage = t('module_calibration_required_no_pipette_attached') + else if (calibratePipetteRequired) + bannerMessage = t('module_calibration_required_no_pipette_calibrated') else if (updatePipetteFWRequired) bannerMessage = t('module_calibration_required_update_pipette_FW') else bannerMessage = t('module_calibration_required') hyperlinkText = - !attachPipetteRequired && !updatePipetteFWRequired && !isTooHot + !attachPipetteRequired && + !updatePipetteFWRequired && + !isTooHot && + !calibratePipetteRequired ? t('calibrate_now') : '' } else { diff --git a/app/src/organisms/Devices/InstrumentsAndModules.tsx b/app/src/organisms/Devices/InstrumentsAndModules.tsx index ef6cb2a7f60..25007ca3d87 100644 --- a/app/src/organisms/Devices/InstrumentsAndModules.tsx +++ b/app/src/organisms/Devices/InstrumentsAndModules.tsx @@ -105,6 +105,8 @@ export function InstrumentsAndModules({ attachedPipettes?.left ?? null ) const attachPipetteRequired = + attachedLeftPipette == null && attachedRightPipette == null + const calibratePipetteRequired = attachedLeftPipette?.data?.calibratedOffset?.last_modified == null && attachedRightPipette?.data?.calibratedOffset?.last_modified == null const updatePipetteFWRequired = @@ -240,6 +242,7 @@ export function InstrumentsAndModules({ module={module} isLoadedInRun={false} attachPipetteRequired={attachPipetteRequired} + calibratePipetteRequired={calibratePipetteRequired} updatePipetteFWRequired={updatePipetteFWRequired} /> ))} @@ -281,6 +284,7 @@ export function InstrumentsAndModules({ module={module} isLoadedInRun={false} attachPipetteRequired={attachPipetteRequired} + calibratePipetteRequired={calibratePipetteRequired} updatePipetteFWRequired={updatePipetteFWRequired} /> ))} diff --git a/app/src/organisms/Devices/ProtocolRun/ProtocolRunModuleControls.tsx b/app/src/organisms/Devices/ProtocolRun/ProtocolRunModuleControls.tsx index f8dcee3d93b..107db017315 100644 --- a/app/src/organisms/Devices/ProtocolRun/ProtocolRunModuleControls.tsx +++ b/app/src/organisms/Devices/ProtocolRun/ProtocolRunModuleControls.tsx @@ -15,6 +15,7 @@ import type { BadPipette, PipetteData } from '@opentrons/api-client' interface PipetteStatus { attachPipetteRequired: boolean + calibratePipetteRequired: boolean updatePipetteFWRequired: boolean } @@ -51,9 +52,16 @@ const usePipetteIsReady = (): PipetteStatus => { const attachPipetteRequired = attachedLeftPipette == null && attachedRightPipette == null + const calibratePipetteRequired = + attachedLeftPipette?.data.calibratedOffset?.last_modified == null && + attachedRightPipette?.data.calibratedOffset?.last_modified == null const updatePipetteFWRequired = leftPipetteRequiresFWUpdate != null || rightPipetteFWRequired != null - return { attachPipetteRequired, updatePipetteFWRequired } + return { + attachPipetteRequired, + calibratePipetteRequired, + updatePipetteFWRequired, + } } interface ProtocolRunModuleControlsProps { @@ -67,7 +75,11 @@ export const ProtocolRunModuleControls = ({ }: ProtocolRunModuleControlsProps): JSX.Element => { const { t } = useTranslation('protocol_details') - const { attachPipetteRequired, updatePipetteFWRequired } = usePipetteIsReady() + const { + attachPipetteRequired, + calibratePipetteRequired, + updatePipetteFWRequired, + } = usePipetteIsReady() const moduleRenderInfoForProtocolById = useModuleRenderInfoForProtocolById( runId @@ -115,6 +127,7 @@ export const ProtocolRunModuleControls = ({ slotName={module.slotName} isLoadedInRun={true} attachPipetteRequired={attachPipetteRequired} + calibratePipetteRequired={calibratePipetteRequired} updatePipetteFWRequired={updatePipetteFWRequired} /> ) : null @@ -135,6 +148,7 @@ export const ProtocolRunModuleControls = ({ slotName={module.slotName} isLoadedInRun={true} attachPipetteRequired={attachPipetteRequired} + calibratePipetteRequired={calibratePipetteRequired} updatePipetteFWRequired={updatePipetteFWRequired} /> ) : null diff --git a/app/src/organisms/ModuleCard/__tests__/ModuleCard.test.tsx b/app/src/organisms/ModuleCard/__tests__/ModuleCard.test.tsx index 599732ad763..31e0f92cd31 100644 --- a/app/src/organisms/ModuleCard/__tests__/ModuleCard.test.tsx +++ b/app/src/organisms/ModuleCard/__tests__/ModuleCard.test.tsx @@ -227,6 +227,7 @@ describe('ModuleCard', () => { robotName: mockRobot.name, isLoadedInRun: false, attachPipetteRequired: false, + calibratePipetteRequired: false, updatePipetteFWRequired: false, } diff --git a/app/src/organisms/ModuleCard/index.tsx b/app/src/organisms/ModuleCard/index.tsx index eb42bf67f39..2d76c99c97f 100644 --- a/app/src/organisms/ModuleCard/index.tsx +++ b/app/src/organisms/ModuleCard/index.tsx @@ -79,6 +79,7 @@ interface ModuleCardProps { robotName: string isLoadedInRun: boolean attachPipetteRequired: boolean + calibratePipetteRequired: boolean updatePipetteFWRequired: boolean runId?: string slotName?: string @@ -93,6 +94,7 @@ export const ModuleCard = (props: ModuleCardProps): JSX.Element | null => { runId, slotName, attachPipetteRequired, + calibratePipetteRequired, updatePipetteFWRequired, } = props const dispatch = useDispatch() @@ -125,7 +127,9 @@ export const ModuleCard = (props: ModuleCardProps): JSX.Element | null => { }) const requireModuleCalibration = module.moduleOffset?.last_modified == null const isPipetteReady = - (!attachPipetteRequired ?? false) && (!updatePipetteFWRequired ?? false) + (!attachPipetteRequired ?? false) && + (!calibratePipetteRequired ?? false) && + (!updatePipetteFWRequired ?? false) const latestRequestId = last(requestIds) const latestRequest = useSelector(state => latestRequestId ? getRequestById(state, latestRequestId) : null @@ -303,6 +307,7 @@ export const ModuleCard = (props: ModuleCardProps): JSX.Element | null => { /> )} {attachPipetteRequired != null && + calibratePipetteRequired != null && updatePipetteFWRequired != null && requireModuleCalibration && !isPending ? ( @@ -313,6 +318,7 @@ export const ModuleCard = (props: ModuleCardProps): JSX.Element | null => { setShowBanner={() => null} handleUpdateClick={handleCalibrateClick} attachPipetteRequired={attachPipetteRequired} + calibratePipetteRequired={calibratePipetteRequired} updatePipetteFWRequired={updatePipetteFWRequired} isTooHot={isTooHot} /> From c45ed00a74999c93c239bca22d0ab2879941aa03 Mon Sep 17 00:00:00 2001 From: Sanniti Pimpley Date: Mon, 18 Dec 2023 16:18:20 -0500 Subject: [PATCH 27/32] feat(api): raise error if tip tracking is not available for current nozzle configuration (#14231) Raises error when pick_up_tip is called without a location when using pipette nozzle configurations that aren't supported in 7 1 --- .../protocol_api/core/engine/instrument.py | 22 ++++++++++ .../opentrons/protocol_api/core/instrument.py | 4 ++ .../core/legacy/legacy_instrument_core.py | 4 ++ .../legacy_instrument_core.py | 4 ++ .../protocol_api/instrument_context.py | 10 ++++- .../core/engine/test_instrument_core.py | 42 ++++++++++++++++++- .../protocol_api/test_instrument_context.py | 17 ++++++++ 7 files changed, 101 insertions(+), 2 deletions(-) diff --git a/api/src/opentrons/protocol_api/core/engine/instrument.py b/api/src/opentrons/protocol_api/core/engine/instrument.py index 8f593d94cd2..2ffe17d2eac 100644 --- a/api/src/opentrons/protocol_api/core/engine/instrument.py +++ b/api/src/opentrons/protocol_api/core/engine/instrument.py @@ -681,6 +681,28 @@ def get_nozzle_configuration(self) -> NozzleConfigurationType: self._pipette_id ) + def is_tip_tracking_available(self) -> bool: + primary_nozzle = self._engine_client.state.pipettes.get_primary_nozzle( + self._pipette_id + ) + if self.get_nozzle_configuration() == NozzleConfigurationType.FULL: + return True + else: + if self.get_channels() == 96: + # SINGLE configuration with H12 nozzle is technically supported by the + # current tip tracking implementation but we don't do any deck conflict + # checks for it, so we won't provide full support for it yet. + return ( + self.get_nozzle_configuration() == NozzleConfigurationType.COLUMN + and primary_nozzle == "A12" + ) + if self.get_channels() == 8: + return ( + self.get_nozzle_configuration() == NozzleConfigurationType.SINGLE + and primary_nozzle == "H1" + ) + return False + def set_flow_rate( self, aspirate: Optional[float] = None, diff --git a/api/src/opentrons/protocol_api/core/instrument.py b/api/src/opentrons/protocol_api/core/instrument.py index 1a54ddb892f..6c034adb4a5 100644 --- a/api/src/opentrons/protocol_api/core/instrument.py +++ b/api/src/opentrons/protocol_api/core/instrument.py @@ -274,5 +274,9 @@ def configure_nozzle_layout( """ ... + def is_tip_tracking_available(self) -> bool: + """Return whether auto tip tracking is available for the pipette's current nozzle configuration.""" + ... + InstrumentCoreType = TypeVar("InstrumentCoreType", bound=AbstractInstrument[Any]) diff --git a/api/src/opentrons/protocol_api/core/legacy/legacy_instrument_core.py b/api/src/opentrons/protocol_api/core/legacy/legacy_instrument_core.py index b91a8821c97..f70540534af 100644 --- a/api/src/opentrons/protocol_api/core/legacy/legacy_instrument_core.py +++ b/api/src/opentrons/protocol_api/core/legacy/legacy_instrument_core.py @@ -547,3 +547,7 @@ def configure_nozzle_layout( def get_active_channels(self) -> int: """This will never be called because it was added in API 2.16.""" assert False, "get_active_channels only supported in API 2.16 & later" + + def is_tip_tracking_available(self) -> bool: + # Tip tracking is always available in legacy context + return True diff --git a/api/src/opentrons/protocol_api/core/legacy_simulator/legacy_instrument_core.py b/api/src/opentrons/protocol_api/core/legacy_simulator/legacy_instrument_core.py index 549275c3983..cd1c3b84a5d 100644 --- a/api/src/opentrons/protocol_api/core/legacy_simulator/legacy_instrument_core.py +++ b/api/src/opentrons/protocol_api/core/legacy_simulator/legacy_instrument_core.py @@ -465,3 +465,7 @@ def configure_nozzle_layout( def get_active_channels(self) -> int: """This will never be called because it was added in API 2.16.""" assert False, "get_active_channels only supported in API 2.16 & later" + + def is_tip_tracking_available(self) -> bool: + # Tip tracking is always available in legacy context + return True diff --git a/api/src/opentrons/protocol_api/instrument_context.py b/api/src/opentrons/protocol_api/instrument_context.py index 29a8114e364..3ab9a6d8cbe 100644 --- a/api/src/opentrons/protocol_api/instrument_context.py +++ b/api/src/opentrons/protocol_api/instrument_context.py @@ -6,6 +6,7 @@ from opentrons_shared_data.errors.exceptions import ( CommandPreconditionViolated, CommandParameterLimitViolated, + UnexpectedTipRemovalError, ) from opentrons.legacy_broker import LegacyBroker from opentrons.hardware_control.dev_types import PipetteDict @@ -26,7 +27,6 @@ requires_version, APIVersionError, ) -from opentrons_shared_data.errors.exceptions import UnexpectedTipRemovalError from .core.common import InstrumentCore, ProtocolCore from .core.engine import ENGINE_CORE_API_VERSION @@ -860,6 +860,14 @@ def pick_up_tip( ) if location is None: + if not self._core.is_tip_tracking_available(): + raise CommandPreconditionViolated( + "Automatic tip tracking is not available for the current pipette" + " nozzle configuration. We suggest switching to a configuration" + " that supports automatic tip tracking or specifying the exact tip" + " to pick up." + ) + tip_rack, well = labware.next_available_tip( starting_tip=self.starting_tip, tip_racks=self.tip_racks, diff --git a/api/tests/opentrons/protocol_api/core/engine/test_instrument_core.py b/api/tests/opentrons/protocol_api/core/engine/test_instrument_core.py index 78c2411174f..d87812f9ad0 100644 --- a/api/tests/opentrons/protocol_api/core/engine/test_instrument_core.py +++ b/api/tests/opentrons/protocol_api/core/engine/test_instrument_core.py @@ -1,5 +1,5 @@ """Test for the ProtocolEngine-based instrument API core.""" -from typing import cast, Optional +from typing import cast, Optional, Union import pytest from decoy import Decoy @@ -8,6 +8,7 @@ from opentrons.hardware_control import SyncHardwareAPI from opentrons.hardware_control.dev_types import PipetteDict +from opentrons.hardware_control.nozzle_manager import NozzleConfigurationType from opentrons.protocol_engine import ( DeckPoint, LoadedPipette, @@ -1011,3 +1012,42 @@ def test_configure_nozzle_layout( decoy.verify( mock_engine_client.configure_nozzle_layout(subject._pipette_id, expected_model) ) + + +@pytest.mark.parametrize( + argnames=["pipette_channels", "nozzle_layout", "primary_nozzle", "expected_result"], + argvalues=[ + (96, NozzleConfigurationType.FULL, "A1", True), + (96, NozzleConfigurationType.FULL, None, True), + (96, NozzleConfigurationType.ROW, "A1", False), + (96, NozzleConfigurationType.COLUMN, "A1", False), + (96, NozzleConfigurationType.COLUMN, "A12", True), + (96, NozzleConfigurationType.SINGLE, "H12", False), + (96, NozzleConfigurationType.SINGLE, "A1", False), + (8, NozzleConfigurationType.FULL, "A1", True), + (8, NozzleConfigurationType.FULL, None, True), + (8, NozzleConfigurationType.SINGLE, "H1", True), + (8, NozzleConfigurationType.SINGLE, "A1", False), + (1, NozzleConfigurationType.FULL, None, True), + ], +) +def test_is_tip_tracking_available( + decoy: Decoy, + mock_engine_client: EngineClient, + subject: InstrumentCore, + pipette_channels: int, + nozzle_layout: NozzleConfigurationType, + primary_nozzle: Union[str, None], + expected_result: bool, +) -> None: + """It should return whether tip tracking is available based on nozzle configuration.""" + decoy.when( + mock_engine_client.state.tips.get_pipette_channels(subject.pipette_id) + ).then_return(pipette_channels) + decoy.when( + mock_engine_client.state.pipettes.get_nozzle_layout_type(subject.pipette_id) + ).then_return(nozzle_layout) + decoy.when( + mock_engine_client.state.pipettes.get_primary_nozzle(subject.pipette_id) + ).then_return(primary_nozzle) + assert subject.is_tip_tracking_available() == expected_result diff --git a/api/tests/opentrons/protocol_api/test_instrument_context.py b/api/tests/opentrons/protocol_api/test_instrument_context.py index 328aed5b01f..78369be3f95 100644 --- a/api/tests/opentrons/protocol_api/test_instrument_context.py +++ b/api/tests/opentrons/protocol_api/test_instrument_context.py @@ -600,6 +600,7 @@ def test_pick_up_from_associated_tip_racks( mock_well = decoy.mock(cls=Well) top_location = Location(point=Point(1, 2, 3), labware=mock_well) + decoy.when(mock_instrument_core.is_tip_tracking_available()).then_return(True) decoy.when(mock_instrument_core.get_active_channels()).then_return(123) decoy.when( labware.next_available_tip( @@ -626,6 +627,22 @@ def test_pick_up_from_associated_tip_racks( ) +def test_pick_up_fails_when_tip_tracking_unavailable( + decoy: Decoy, mock_instrument_core: InstrumentCore, subject: InstrumentContext +) -> None: + """It should raise an error if automatic tip tracking is not available..""" + mock_tip_rack_1 = decoy.mock(cls=Labware) + + decoy.when(mock_instrument_core.is_tip_tracking_available()).then_return(False) + decoy.when(mock_instrument_core.get_active_channels()).then_return(123) + + subject.tip_racks = [mock_tip_rack_1] + with pytest.raises( + CommandPreconditionViolated, match="Automatic tip tracking is not available" + ): + subject.pick_up_tip() + + def test_drop_tip_to_well( decoy: Decoy, mock_instrument_core: InstrumentCore, subject: InstrumentContext ) -> None: From 45b52ed9af06960e38692e760c4e17667544a397 Mon Sep 17 00:00:00 2001 From: Shlok Amin Date: Mon, 18 Dec 2023 16:39:07 -0500 Subject: [PATCH 28/32] feat(app): add loading state to ODD protocol list (#14238) closes RQA-2137 --- .../pages/ProtocolDashboard/ProtocolCard.tsx | 40 ++++++++++++++----- .../__tests__/ProtocolCard.test.tsx | 20 ++++------ 2 files changed, 37 insertions(+), 23 deletions(-) diff --git a/app/src/pages/ProtocolDashboard/ProtocolCard.tsx b/app/src/pages/ProtocolDashboard/ProtocolCard.tsx index e52af66025a..fca8db7fe72 100644 --- a/app/src/pages/ProtocolDashboard/ProtocolCard.tsx +++ b/app/src/pages/ProtocolDashboard/ProtocolCard.tsx @@ -15,6 +15,7 @@ import { DIRECTION_ROW, Flex, Icon, + SIZE_2, SPACING, TYPOGRAPHY, useLongPress, @@ -35,6 +36,8 @@ import type { UseLongPressResult } from '@opentrons/components' import type { ProtocolResource } from '@opentrons/shared-data' import type { ModalHeaderBaseProps } from '../../molecules/Modal/types' +const REFETCH_INTERVAL = 5000 + export function ProtocolCard(props: { protocol: ProtocolResource longPress: React.Dispatch> @@ -61,22 +64,25 @@ export function ProtocolCard(props: { const queryClient = useQueryClient() const host = useHost() - const { - data: mostRecentAnalysis, - } = useProtocolAnalysisAsDocumentQuery( + const { data: mostRecentAnalysis } = useProtocolAnalysisAsDocumentQuery( protocol.id, last(protocol.analysisSummaries)?.id ?? null, - { enabled: protocol != null } + { + enabled: protocol != null, + refetchInterval: analysisData => + analysisData == null ? REFETCH_INTERVAL : false, + } ) const isFailedAnalysis = - (mostRecentAnalysis == null || - (mostRecentAnalysis != null && - 'result' in mostRecentAnalysis && - (mostRecentAnalysis.result === 'error' || - mostRecentAnalysis.result === 'not-ok'))) ?? + (mostRecentAnalysis != null && + 'result' in mostRecentAnalysis && + (mostRecentAnalysis.result === 'error' || + mostRecentAnalysis.result === 'not-ok')) ?? false + const isPendingAnalysis = mostRecentAnalysis == null + const handleProtocolClick = ( longpress: UseLongPressResult, protocolId: string @@ -157,6 +163,16 @@ export function ProtocolCard(props: { ref={longpress.ref} css={PUSHED_STATE_STYLE} > + {isPendingAnalysis ? ( + + ) : null} ) : null} - + {protocolName} diff --git a/app/src/pages/ProtocolDashboard/__tests__/ProtocolCard.test.tsx b/app/src/pages/ProtocolDashboard/__tests__/ProtocolCard.test.tsx index 1fbf3feb798..20962592973 100644 --- a/app/src/pages/ProtocolDashboard/__tests__/ProtocolCard.test.tsx +++ b/app/src/pages/ProtocolDashboard/__tests__/ProtocolCard.test.tsx @@ -1,5 +1,5 @@ import * as React from 'react' -import { fireEvent } from '@testing-library/react' +import { fireEvent, screen } from '@testing-library/react' import { MemoryRouter } from 'react-router-dom' import { UseQueryResult } from 'react-query' import { useProtocolAnalysisAsDocumentQuery } from '@opentrons/react-api-client' @@ -126,22 +126,16 @@ describe('ProtocolCard', () => { getByText('Delete protocol') }) - it('should display the analysis failed error modal when clicking on the protocol when doing a long pressing - undefined case', async () => { + it('should display a loading spinner when analysis is pending', async () => { mockUseProtocolAnalysisAsDocumentQuery.mockReturnValue({ - data: undefined as any, + data: null as any, } as UseQueryResult) - const [{ getByText, getByLabelText }] = render() - const name = getByText('yay mock protocol') + render() + const name = screen.getByText('yay mock protocol') fireEvent.mouseDown(name) jest.advanceTimersByTime(1005) expect(props.longPress).toHaveBeenCalled() - getByLabelText('failedAnalysis_icon') - getByText('Failed analysis') - getByText('yay mock protocol').click() - getByText('Protocol analysis failed') - getByText( - 'Delete the protocol, make changes to address the error, and resend the protocol to this robot from the Opentrons App.' - ) - getByText('Delete protocol') + screen.getByLabelText('Protocol is loading') + screen.getByText('yay mock protocol').click() }) }) From 0ea0fb8a4787f8606d387baee979b6455ba424b7 Mon Sep 17 00:00:00 2001 From: Max Marrone Date: Mon, 18 Dec 2023 16:50:01 -0500 Subject: [PATCH 29/32] feat(protocol-engine): Allow moving to addressable areas without descending (#14234) --- .../commands/move_to_addressable_area.py | 9 +++ .../protocol_engine/execution/movement.py | 2 + .../opentrons/protocol_engine/state/motion.py | 18 ++++- .../commands/test_move_to_addressable_area.py | 2 + .../execution/test_movement_handler.py | 2 + .../protocol_engine/state/test_motion_view.py | 72 ++++++++++++++++++- shared-data/command/schemas/8.json | 6 ++ shared-data/command/types/gantry.ts | 1 + .../protocol/models/protocol_schema_v8.py | 1 + 9 files changed, 110 insertions(+), 3 deletions(-) diff --git a/api/src/opentrons/protocol_engine/commands/move_to_addressable_area.py b/api/src/opentrons/protocol_engine/commands/move_to_addressable_area.py index b817eae0f5c..7dfc0b53895 100644 --- a/api/src/opentrons/protocol_engine/commands/move_to_addressable_area.py +++ b/api/src/opentrons/protocol_engine/commands/move_to_addressable_area.py @@ -54,6 +54,14 @@ class MoveToAddressableAreaParams(PipetteIdMixin, MovementMixin): AddressableOffsetVector(x=0, y=0, z=0), description="Relative offset of addressable area to move pipette's critical point.", ) + stayAtHighestPossibleZ: bool = Field( + False, + description=( + "If `true`, the pipette will retract to its highest possible height" + " and stay there instead of descending to the destination." + " `minimumZHeight` will be ignored." + ), + ) class MoveToAddressableAreaResult(DestinationPositionResult): @@ -93,6 +101,7 @@ async def execute( force_direct=params.forceDirect, minimum_z_height=params.minimumZHeight, speed=params.speed, + stay_at_highest_possible_z=params.stayAtHighestPossibleZ, ) return MoveToAddressableAreaResult(position=DeckPoint(x=x, y=y, z=z)) diff --git a/api/src/opentrons/protocol_engine/execution/movement.py b/api/src/opentrons/protocol_engine/execution/movement.py index 029d31a7c17..8e65986fd07 100644 --- a/api/src/opentrons/protocol_engine/execution/movement.py +++ b/api/src/opentrons/protocol_engine/execution/movement.py @@ -146,6 +146,7 @@ async def move_to_addressable_area( force_direct: bool = False, minimum_z_height: Optional[float] = None, speed: Optional[float] = None, + stay_at_highest_possible_z: bool = False, ) -> Point: """Move to a specific addressable area.""" # Check for presence of heater shakers on deck, and if planned @@ -191,6 +192,7 @@ async def move_to_addressable_area( max_travel_z=max_travel_z, force_direct=force_direct, minimum_z_height=minimum_z_height, + stay_at_max_travel_z=stay_at_highest_possible_z, ) speed = self._state_store.pipettes.get_movement_speed( diff --git a/api/src/opentrons/protocol_engine/state/motion.py b/api/src/opentrons/protocol_engine/state/motion.py index 8735b5d492e..edd4cca2cca 100644 --- a/api/src/opentrons/protocol_engine/state/motion.py +++ b/api/src/opentrons/protocol_engine/state/motion.py @@ -149,6 +149,7 @@ def get_movement_waypoints_to_addressable_area( max_travel_z: float, force_direct: bool = False, minimum_z_height: Optional[float] = None, + stay_at_max_travel_z: bool = False, ) -> List[motion_planning.Waypoint]: """Calculate waypoints to a destination that's specified as an addressable area.""" location = self._pipettes.get_current_location() @@ -158,7 +159,22 @@ def get_movement_waypoints_to_addressable_area( addressable_area_name ) ) - destination = base_destination + Point(x=offset.x, y=offset.y, z=offset.z) + if stay_at_max_travel_z: + base_destination_at_max_z = Point( + base_destination.x, + base_destination.y, + # HACK(mm, 2023-12-18): We want to travel exactly at max_travel_z, but + # motion_planning.get_waypoints() won't let us--the highest we can go is this margin + # beneath max_travel_z. Investigate why motion_planning.get_waypoints() does not + # let us travel at max_travel_z, and whether it's safe to make it do that. + # Possibly related: https://github.com/Opentrons/opentrons/pull/6882#discussion_r514248062 + max_travel_z - motion_planning.waypoints.MINIMUM_Z_MARGIN, + ) + destination = base_destination_at_max_z + Point( + offset.x, offset.y, offset.z + ) + else: + destination = base_destination + Point(offset.x, offset.y, offset.z) # TODO(jbl 11-28-2023) This may need to change for partial tip configurations on a 96 destination_cp = CriticalPoint.XY_CENTER diff --git a/api/tests/opentrons/protocol_engine/commands/test_move_to_addressable_area.py b/api/tests/opentrons/protocol_engine/commands/test_move_to_addressable_area.py index b18e9ba7c97..20515bc12c4 100644 --- a/api/tests/opentrons/protocol_engine/commands/test_move_to_addressable_area.py +++ b/api/tests/opentrons/protocol_engine/commands/test_move_to_addressable_area.py @@ -30,6 +30,7 @@ async def test_move_to_addressable_area_implementation( forceDirect=True, minimumZHeight=4.56, speed=7.89, + stayAtHighestPossibleZ=True, ) decoy.when( @@ -40,6 +41,7 @@ async def test_move_to_addressable_area_implementation( force_direct=True, minimum_z_height=4.56, speed=7.89, + stay_at_highest_possible_z=True, ) ).then_return(Point(x=9, y=8, z=7)) diff --git a/api/tests/opentrons/protocol_engine/execution/test_movement_handler.py b/api/tests/opentrons/protocol_engine/execution/test_movement_handler.py index 13d7da9f9e5..cd4345f7f67 100644 --- a/api/tests/opentrons/protocol_engine/execution/test_movement_handler.py +++ b/api/tests/opentrons/protocol_engine/execution/test_movement_handler.py @@ -353,6 +353,7 @@ async def test_move_to_addressable_area( max_travel_z=42.0, force_direct=True, minimum_z_height=12.3, + stay_at_max_travel_z=True, ) ).then_return( [Waypoint(Point(1, 2, 3), CriticalPoint.XY_CENTER), Waypoint(Point(4, 5, 6))] @@ -376,6 +377,7 @@ async def test_move_to_addressable_area( force_direct=True, minimum_z_height=12.3, speed=45.6, + stay_at_highest_possible_z=True, ) assert result == Point(x=4, y=5, z=6) diff --git a/api/tests/opentrons/protocol_engine/state/test_motion_view.py b/api/tests/opentrons/protocol_engine/state/test_motion_view.py index 750eb7b9f4b..0b76a55f7af 100644 --- a/api/tests/opentrons/protocol_engine/state/test_motion_view.py +++ b/api/tests/opentrons/protocol_engine/state/test_motion_view.py @@ -502,11 +502,9 @@ def test_get_movement_waypoints_to_well_raises( def test_get_movement_waypoints_to_addressable_area( decoy: Decoy, - labware_view: LabwareView, pipette_view: PipetteView, addressable_area_view: AddressableAreaView, geometry_view: GeometryView, - mock_module_view: ModuleView, subject: MotionView, ) -> None: """It should call get_waypoints() with the correct args to move to an addressable area.""" @@ -561,6 +559,76 @@ def test_get_movement_waypoints_to_addressable_area( assert result == waypoints +def test_get_movement_waypoints_to_addressable_area_stay_at_max_travel_z( + decoy: Decoy, + pipette_view: PipetteView, + addressable_area_view: AddressableAreaView, + geometry_view: GeometryView, + subject: MotionView, +) -> None: + """It should call get_waypoints() with the correct args to move to an addressable area. + + This is the variant where we pass stay_at_max_travel_z=True to the subject. + This should affect the dest argument of get_waypoints(). + """ + location = CurrentAddressableArea(pipette_id="123", addressable_area_name="abc") + + decoy.when(pipette_view.get_current_location()).then_return(location) + decoy.when( + addressable_area_view.get_addressable_area_move_to_location("area-name") + ).then_return(Point(x=3, y=3, z=3)) + decoy.when(geometry_view.get_all_obstacle_highest_z()).then_return(42) + + decoy.when( + addressable_area_view.get_addressable_area_base_slot("area-name") + ).then_return(DeckSlotName.SLOT_2) + + decoy.when( + geometry_view.get_extra_waypoints(location, DeckSlotName.SLOT_2) + ).then_return([]) + + waypoints = [ + motion_planning.Waypoint( + position=Point(1, 2, 3), critical_point=CriticalPoint.XY_CENTER + ), + motion_planning.Waypoint( + position=Point(4, 5, 6), critical_point=CriticalPoint.MOUNT + ), + ] + + decoy.when( + motion_planning.get_waypoints( + move_type=motion_planning.MoveType.DIRECT, + origin=Point(x=1, y=2, z=3), + origin_cp=CriticalPoint.MOUNT, + max_travel_z=1337, + min_travel_z=123, + dest=Point( + x=4, + y=5, + # The max_travel_z arg passed to the subject, plus the offset passed to the subject, + # minus a 1 mm margin as a hack--see comments in the subject. + z=1337 + 3 - 1, + ), + dest_cp=CriticalPoint.XY_CENTER, + xy_waypoints=[], + ) + ).then_return(waypoints) + + result = subject.get_movement_waypoints_to_addressable_area( + addressable_area_name="area-name", + offset=AddressableOffsetVector(x=1, y=2, z=3), + origin=Point(x=1, y=2, z=3), + origin_cp=CriticalPoint.MOUNT, + max_travel_z=1337, + force_direct=True, + minimum_z_height=123, + stay_at_max_travel_z=True, + ) + + assert result == waypoints + + @pytest.mark.parametrize( ("direct", "expected_move_type"), [ diff --git a/shared-data/command/schemas/8.json b/shared-data/command/schemas/8.json index c95c466db5e..984fe40cb36 100644 --- a/shared-data/command/schemas/8.json +++ b/shared-data/command/schemas/8.json @@ -1965,6 +1965,12 @@ "$ref": "#/definitions/AddressableOffsetVector" } ] + }, + "stayAtHighestPossibleZ": { + "title": "Stayathighestpossiblez", + "description": "If `true`, the pipette will retract to its highest possible height and stay there instead of descending to the destination. `minimumZHeight` will be ignored.", + "default": false, + "type": "boolean" } }, "required": ["pipetteId", "addressableAreaName"] diff --git a/shared-data/command/types/gantry.ts b/shared-data/command/types/gantry.ts index 435188d948d..c016cf66678 100644 --- a/shared-data/command/types/gantry.ts +++ b/shared-data/command/types/gantry.ts @@ -180,4 +180,5 @@ export interface MoveToAddressableAreaParams { speed?: number minimumZHeight?: number forceDirect?: boolean + stayAtHighestPossibleZ?: boolean } diff --git a/shared-data/python/opentrons_shared_data/protocol/models/protocol_schema_v8.py b/shared-data/python/opentrons_shared_data/protocol/models/protocol_schema_v8.py index 5a2804ec6e0..5ec17baa46c 100644 --- a/shared-data/python/opentrons_shared_data/protocol/models/protocol_schema_v8.py +++ b/shared-data/python/opentrons_shared_data/protocol/models/protocol_schema_v8.py @@ -69,6 +69,7 @@ class Params(BaseModel): # schema v8 add-ons addressableAreaName: Optional[str] configurationParams: Optional[NozzleConfigurationParams] + stayAtHighestPossibleZ: Optional[bool] class Command(BaseModel): From 6417cf74c1d83e50841f8f484be237a2958cfee1 Mon Sep 17 00:00:00 2001 From: Nick Diehl <47604184+ncdiehl11@users.noreply.github.com> Date: Mon, 18 Dec 2023 16:50:38 -0500 Subject: [PATCH 30/32] feat(app): add gradient scrim to ODD protocol command list (#14232) * feat(app): add gradient scrim to ODD protocol command list closes RQA-1630 --- .../pages/OnDeviceDisplay/RunningProtocol.tsx | 43 +++++++++++++------ 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/app/src/pages/OnDeviceDisplay/RunningProtocol.tsx b/app/src/pages/OnDeviceDisplay/RunningProtocol.tsx index 66f34f4f4a6..48a55430427 100644 --- a/app/src/pages/OnDeviceDisplay/RunningProtocol.tsx +++ b/app/src/pages/OnDeviceDisplay/RunningProtocol.tsx @@ -1,6 +1,6 @@ import * as React from 'react' import { useParams } from 'react-router-dom' -import styled from 'styled-components' +import styled, { css } from 'styled-components' import { useSelector } from 'react-redux' import { @@ -12,6 +12,8 @@ import { JUSTIFY_CENTER, OVERFLOW_HIDDEN, POSITION_RELATIVE, + POSITION_ABSOLUTE, + ALIGN_FLEX_END, SPACING, useSwipe, } from '@opentrons/components' @@ -216,18 +218,33 @@ export function RunningProtocol(): JSX.Element { } /> ) : ( - + <> + + + ) ) : ( From e8fc541dc7b10cfd2fbefccc609a59291ea3052c Mon Sep 17 00:00:00 2001 From: Ed Cormany Date: Mon, 18 Dec 2023 17:02:11 -0500 Subject: [PATCH 31/32] docs(api): corrections to docstrings involving `TrashBin`s (#14194) --------- Co-authored-by: Max Marrone --- .../opentrons/protocol_api/instrument_context.py | 16 +++++++++------- .../opentrons/protocol_api/protocol_context.py | 12 ++++++++---- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/api/src/opentrons/protocol_api/instrument_context.py b/api/src/opentrons/protocol_api/instrument_context.py index 3ab9a6d8cbe..0a1743e382f 100644 --- a/api/src/opentrons/protocol_api/instrument_context.py +++ b/api/src/opentrons/protocol_api/instrument_context.py @@ -958,7 +958,7 @@ def drop_tip( See :ref:`pipette-drop-tip` for examples. If no location is passed (e.g. ``pipette.drop_tip()``), the pipette will drop - the attached tip into its default :py:attr:`trash_container`. + the attached tip into its :py:attr:`trash_container`. Starting with API version 2.15, if the trash container is the default fixed trash, the API will instruct the pipette to drop tips in different locations @@ -1538,12 +1538,14 @@ def trash_container(self) -> Union[labware.Labware, TrashBin, WasteChute]: This is the property used to determine where to drop tips and blow out liquids when calling :py:meth:`drop_tip` or :py:meth:`blow_out` without arguments. - On a Flex running a protocol with API version 2.16 or higher, ``trash_container`` is - the first ``TrashBin`` or ``WasteChute`` object loaded in the protocol. - On a Flex running a protocol with API version 2.15, ``trash_container`` is - a single-well fixed trash labware in slot D3. - On a an OT-2, ``trash_container`` is always a single-well fixed trash labware - in slot 12. + You can set this to a :py:obj:`Labware`, :py:obj:`TrashBin`, or :py:obj:`WasteChute`. + + The default value depends on the robot type and API version: + + - :py:obj:`ProtocolContext.fixed_trash`, if it exists. + - Otherwise, the first item previously loaded with + :py:obj:`ProtocolContext.load_trash_bin()` or + :py:obj:`ProtocolContext.load_waste_chute()`. .. versionchanged:: 2.16 Added support for ``TrashBin`` and ``WasteChute`` objects. diff --git a/api/src/opentrons/protocol_api/protocol_context.py b/api/src/opentrons/protocol_api/protocol_context.py index 33a2e0b07de..02293be7298 100644 --- a/api/src/opentrons/protocol_api/protocol_context.py +++ b/api/src/opentrons/protocol_api/protocol_context.py @@ -1055,12 +1055,16 @@ def deck(self) -> Deck: @property # type: ignore @requires_version(2, 0) def fixed_trash(self) -> Union[Labware, TrashBin]: - """The trash fixed to slot 12 of the robot deck. + """The trash fixed to slot 12 of an OT-2's deck. - In API Versions prior to 2.16 it has one well and should be accessed like labware in your protocol. - e.g. ``protocol.fixed_trash['A1']`` + In API version 2.15 and earlier, the fixed trash is a :py:class:`.Labware` object with one well. Access it like labware in your protocol. For example, ``protocol.fixed_trash['A1']``. - In API Version 2.16 and above it returns a Trash fixture for OT-2 Protocols. + In API version 2.15 only, Flex protocols have a fixed trash in slot A3. + + In API version 2.16 and later, the fixed trash only exists in OT-2 protocols. It is a :py:class:`.TrashBin` object, which doesn't have any wells. Trying to access ``fixed_trash`` in a Flex protocol will raise an error. See :ref:`configure-trash-bin` for details on using the movable trash in Flex protocols. + + .. versionchanged:: 2.16 + Returns a :py:class:`.TrashBin` object. """ if self._api_version >= APIVersion(2, 16): if self._core.robot_type == "OT-3 Standard": From 2bc83780bc49f3de8acf508ec6e96877aa16e217 Mon Sep 17 00:00:00 2001 From: Jamey H Date: Mon, 18 Dec 2023 17:15:35 -0500 Subject: [PATCH 32/32] refactor(app): remove excessive axis homing in drop tip wizard (#14239) * refactor(app): remove excessive axis homing Now that moveToAddressableArea provides the ability to home to Z axis before moving, let's remove the homing done during the flows but keep still keep the homing behavior at the end of the flow. --- app/src/organisms/DropTipWizard/index.tsx | 131 ++++++---------------- 1 file changed, 32 insertions(+), 99 deletions(-) diff --git a/app/src/organisms/DropTipWizard/index.tsx b/app/src/organisms/DropTipWizard/index.tsx index 80742606d04..e2829b40a61 100644 --- a/app/src/organisms/DropTipWizard/index.tsx +++ b/app/src/organisms/DropTipWizard/index.tsx @@ -45,13 +45,10 @@ import { ChooseLocation } from './ChooseLocation' import { JogToPosition } from './JogToPosition' import { Success } from './Success' -import type { PipetteData, CommandData } from '@opentrons/api-client' +import type { PipetteData } from '@opentrons/api-client' import type { - Coordinates, PipetteModelSpecs, RobotType, - SavePositionRunTimeCommand, - CreateCommand, DeckConfiguration, AddressableAreaName, } from '@opentrons/shared-data' @@ -227,7 +224,6 @@ export const DropTipWizardComponent = ( createMaintenanceRun, handleCleanUpAndClose, chainRunCommands, - // attachedInstrument, isRobotMoving, createRunCommand, setErrorMessage, @@ -261,15 +257,7 @@ export const DropTipWizardComponent = ( const goBack = (): void => { if (createdMaintenanceRunId != null) { - retractAllAxesAndSavePosition() - .then(() => - setCurrentStepIndex( - isFinalStep ? currentStepIndex : currentStepIndex - 1 - ) - ) - .catch((e: Error) => - setErrorMessage(`Error issuing jog command: ${e.message}`) - ) + setCurrentStepIndex(isFinalStep ? currentStepIndex : currentStepIndex - 1) } } @@ -303,103 +291,48 @@ export const DropTipWizardComponent = ( cancel: cancelExit, } = useConditionalConfirm(handleCleanUpAndClose, true) - const retractAllAxesAndSavePosition = (): Promise => { - if (createdMaintenanceRunId == null) - return Promise.reject( - new Error('no maintenance run present to send move commands to') - ) - const commands: CreateCommand[] = [ - { - commandType: 'home' as const, - params: { axes: ['leftZ', 'rightZ', 'x', 'y'] }, - }, - { - commandType: 'savePosition' as const, - params: { - pipetteId: MANAGED_PIPETTE_ID, - failOnNotHomed: false, - }, - }, - ] - return chainRunCommands(createdMaintenanceRunId, commands, false) - .then(responses => { - if (responses.length !== commands.length) { - return Promise.reject( - new Error('Not all commands executed successfully') - ) - } - const currentPosition = (responses[responses.length - 1] - .data as SavePositionRunTimeCommand).result?.position - if (currentPosition != null) { - return Promise.resolve(currentPosition) - } else { - return Promise.reject( - new Error('Current position could not be saved') - ) - } - }) - .catch(e => { - setErrorMessage( - `Error retracting x and y axes or saving position: ${e.message}` - ) - return null - }) - } - const moveToAddressableArea = ( addressableArea: AddressableAreaName - ): Promise => { + ): Promise => { if (createdMaintenanceRunId == null) { return Promise.reject( new Error('no maintenance run present to send move commands to') ) } - return retractAllAxesAndSavePosition() - .then(currentPosition => { - const addressableAreaFromConfig = getAddressableAreaFromConfig( - addressableArea, - deckConfig, - instrumentModelSpecs.channels, - robotType - ) - - const zOffset = - addressableAreaFromConfig === addressableArea && - addressableAreaFromConfig !== 'fixedTrash' - ? (currentPosition as Coordinates).z - 10 - : 0 + const addressableAreaFromConfig = getAddressableAreaFromConfig( + addressableArea, + deckConfig, + instrumentModelSpecs.channels, + robotType + ) - if (currentPosition != null && addressableAreaFromConfig != null) { - return chainRunCommands( - createdMaintenanceRunId, - [ - { - commandType: 'moveToAddressableArea', - params: { - pipetteId: MANAGED_PIPETTE_ID, - addressableAreaName: addressableAreaFromConfig, - offset: { x: 0, y: 0, z: zOffset }, - }, - }, - ], - true - ).then(commandData => { - const error = commandData[0].data.error - if (error != null) { - setErrorMessage(`error moving to position: ${error.detail}`) - } - return null - }) - } else { - setErrorMessage(`error moving to position: invalid addressable area.`) - return null + if (addressableAreaFromConfig != null) { + return chainRunCommands( + createdMaintenanceRunId, + [ + { + commandType: 'moveToAddressableArea', + params: { + pipetteId: MANAGED_PIPETTE_ID, + stayAtHighestPossibleZ: true, + addressableAreaName: addressableAreaFromConfig, + offset: { x: 0, y: 0, z: 0 }, + }, + }, + ], + true + ).then(commandData => { + const error = commandData[0].data.error + if (error != null) { + setErrorMessage(`error moving to position: ${error.detail}`) } - }) - .catch(e => { - setErrorMessage(`error moving to position: ${e.message}`) return null }) + } else { + setErrorMessage(`error moving to position: invalid addressable area.`) + return Promise.resolve(null) + } } let modalContent: JSX.Element =
UNASSIGNED STEP