diff --git a/api-client/src/runs/commands/types.ts b/api-client/src/runs/commands/types.ts index 8c5517a1fe3..215756ede66 100644 --- a/api-client/src/runs/commands/types.ts +++ b/api-client/src/runs/commands/types.ts @@ -1,8 +1,8 @@ import type { RunTimeCommand, RunCommandError } from '@opentrons/shared-data' export interface GetCommandsParams { - cursor: number | null // the index of the command at the center of the window pageLength: number // the number of items to include + cursor?: number } export interface GetRunCommandsParams extends GetCommandsParams { @@ -10,7 +10,7 @@ export interface GetRunCommandsParams extends GetCommandsParams { } export interface GetRunCommandsParamsRequest extends GetCommandsParams { - includeFixitCommands: boolean | null + includeFixitCommands?: boolean } export interface RunCommandErrors { diff --git a/api-client/src/runs/getErrorRecoveryPolicy.ts b/api-client/src/runs/getErrorRecoveryPolicy.ts new file mode 100644 index 00000000000..67a3d0bd93c --- /dev/null +++ b/api-client/src/runs/getErrorRecoveryPolicy.ts @@ -0,0 +1,17 @@ +import { GET, request } from '../request' + +import type { HostConfig } from '../types' +import type { ResponsePromise } from '../request' +import type { ErrorRecoveryPolicyResponse } from './types' + +export function getErrorRecoveryPolicy( + config: HostConfig, + runId: string +): ResponsePromise { + return request( + GET, + `/runs/${runId}/errorRecoveryPolicy`, + null, + config + ) +} diff --git a/api-client/src/runs/index.ts b/api-client/src/runs/index.ts index 183b8f7e4d4..fff1f303543 100644 --- a/api-client/src/runs/index.ts +++ b/api-client/src/runs/index.ts @@ -15,6 +15,7 @@ export * from './createLabwareOffset' export * from './createLabwareDefinition' export * from './constants' export * from './updateErrorRecoveryPolicy' +export * from './getErrorRecoveryPolicy' export * from './types' export type { CreateRunData } from './createRun' diff --git a/api-client/src/runs/types.ts b/api-client/src/runs/types.ts index bf8596b66d7..e41626b6448 100644 --- a/api-client/src/runs/types.ts +++ b/api-client/src/runs/types.ts @@ -204,6 +204,7 @@ export interface UpdateErrorRecoveryPolicyRequest { } export type UpdateErrorRecoveryPolicyResponse = Record +export type ErrorRecoveryPolicyResponse = UpdateErrorRecoveryPolicyRequest /** * Current Run State Data diff --git a/api/release-notes.md b/api/release-notes.md index e41d415d83e..1fdd9a033d9 100644 --- a/api/release-notes.md +++ b/api/release-notes.md @@ -24,6 +24,10 @@ Welcome to the v8.2.0 release of the Opentrons robot software! This release adds - Error recovery no longer causes an `AssertionError` when a Python protocol changes the pipette speed. +### Known Issues + +- You can't downgrade the robot software with an Absorbance Plate Reader attached. Disconnect the module first if you need to downgrade. + --- ## Opentrons Robot Software Changes in 8.1.0 diff --git a/api/src/opentrons/hardware_control/modules/absorbance_reader.py b/api/src/opentrons/hardware_control/modules/absorbance_reader.py index ab6ce1bb22b..ec4a80b7f60 100644 --- a/api/src/opentrons/hardware_control/modules/absorbance_reader.py +++ b/api/src/opentrons/hardware_control/modules/absorbance_reader.py @@ -312,6 +312,8 @@ async def update_device(self, firmware_file_path: str) -> Tuple[bool, str]: log.debug(f"Updating {self.name}: {self.port} with {firmware_file_path}") self._updating = True success, res = await self._driver.update_firmware(firmware_file_path) + # it takes time for the plate reader to re-init after an update. + await asyncio.sleep(10) self._device_info = await self._driver.get_device_info() await self._poller.start() self._updating = False diff --git a/api/src/opentrons/protocol_engine/commands/absorbance_reader/read.py b/api/src/opentrons/protocol_engine/commands/absorbance_reader/read.py index 1ca848858b6..8e8926089f1 100644 --- a/api/src/opentrons/protocol_engine/commands/absorbance_reader/read.py +++ b/api/src/opentrons/protocol_engine/commands/absorbance_reader/read.py @@ -82,7 +82,7 @@ async def execute( # noqa: C901 ) if abs_reader_substate.is_lid_on is False: raise CannotPerformModuleAction( - "Cannot perform Read action on Absorbance Reader with the lid open. Try calling `.close_lid()` first." + "Absorbance Plate Reader can't read a plate with the lid open. Call `close_lid()` first." ) # TODO: we need to return a file ID and increase the file count even when a moduel is not attached diff --git a/api/src/opentrons/protocol_engine/commands/unsafe/unsafe_ungrip_labware.py b/api/src/opentrons/protocol_engine/commands/unsafe/unsafe_ungrip_labware.py index 9674513d749..6f8f5b71fce 100644 --- a/api/src/opentrons/protocol_engine/commands/unsafe/unsafe_ungrip_labware.py +++ b/api/src/opentrons/protocol_engine/commands/unsafe/unsafe_ungrip_labware.py @@ -1,6 +1,8 @@ """Ungrip labware payload, result, and implementaiton.""" from __future__ import annotations + +from opentrons.hardware_control.types import Axis from opentrons.protocol_engine.errors.exceptions import GripperNotAttachedError from pydantic import BaseModel from typing import Optional, Type @@ -46,7 +48,7 @@ async def execute( ot3_hardware_api = ensure_ot3_hardware(self._hardware_api) if not ot3_hardware_api.has_gripper(): raise GripperNotAttachedError("No gripper found to perform ungrip.") - await ot3_hardware_api.ungrip() + await ot3_hardware_api.home([Axis.G]) return SuccessData( public=UnsafeUngripLabwareResult(), ) diff --git a/api/src/opentrons/protocol_engine/state/modules.py b/api/src/opentrons/protocol_engine/state/modules.py index c61d4173ff1..f2f9dc5e8e4 100644 --- a/api/src/opentrons/protocol_engine/state/modules.py +++ b/api/src/opentrons/protocol_engine/state/modules.py @@ -1268,7 +1268,10 @@ def convert_absorbance_reader_data_points( row = chr(ord("A") + i // 12) # Convert index to row (A-H) col = (i % 12) + 1 # Convert index to column (1-12) well_key = f"{row}{col}" - well_map[well_key] = value + truncated_value = float( + "{:.5}".format(str(value)) + ) # Truncate the returned value to the third decimal place + well_map[well_key] = truncated_value return well_map else: raise ValueError( diff --git a/api/tests/opentrons/protocol_engine/commands/unsafe/test_ungrip_labware.py b/api/tests/opentrons/protocol_engine/commands/unsafe/test_ungrip_labware.py index a4eae34a08d..bcd77093abf 100644 --- a/api/tests/opentrons/protocol_engine/commands/unsafe/test_ungrip_labware.py +++ b/api/tests/opentrons/protocol_engine/commands/unsafe/test_ungrip_labware.py @@ -1,6 +1,7 @@ """Test update-position-estimator commands.""" from decoy import Decoy +from opentrons.hardware_control.types import Axis from opentrons.protocol_engine.commands.unsafe.unsafe_ungrip_labware import ( UnsafeUngripLabwareParams, UnsafeUngripLabwareResult, @@ -25,7 +26,7 @@ async def test_ungrip_labware_implementation( assert result == SuccessData(public=UnsafeUngripLabwareResult()) decoy.verify( - await ot3_hardware_api.ungrip(), + await ot3_hardware_api.home([Axis.G]), ) diff --git a/app-shell/build/release-notes.md b/app-shell/build/release-notes.md index a093e29e6ed..9b9231e9709 100644 --- a/app-shell/build/release-notes.md +++ b/app-shell/build/release-notes.md @@ -30,6 +30,10 @@ Welcome to the v8.2.0 release of the Opentrons App! This release adds support fo - Fixed an app crash when performing certain error recovery steps with Python API version 2.15 protocols. +### Known Issues + +- If you attach an Absorbance Plate Reader to _any_ Flex on your local network, you must update all copies of the Opentrons App on the same network to at least v8.1.0. + --- ## Opentrons App Changes in 8.1.0 diff --git a/app/src/assets/images/magnetic_block_gen_1@3x.png b/app/src/assets/images/magnetic_block_gen_1@3x.png new file mode 100644 index 00000000000..860966e07a3 Binary files /dev/null and b/app/src/assets/images/magnetic_block_gen_1@3x.png differ diff --git a/app/src/assets/localization/en/app_settings.json b/app/src/assets/localization/en/app_settings.json index a5edc2c727f..f73ac990fc6 100644 --- a/app/src/assets/localization/en/app_settings.json +++ b/app/src/assets/localization/en/app_settings.json @@ -41,7 +41,7 @@ "enable_dev_tools_description": "Enabling this setting opens Developer Tools on app launch, enables additional logging and gives access to feature flags.", "error_boundary_desktop_app_description": "You need to reload the app. Contact support with the following error message:", "error_boundary_title": "An unknown error has occurred", - "error_recovery_mode": "Error Recovery Mode", + "error_recovery_mode": "Recovery mode", "error_recovery_mode_description": "Pause on protocol errors instead of canceling the run.", "feature_flags": "Feature Flags", "general": "General", diff --git a/app/src/assets/localization/en/error_recovery.json b/app/src/assets/localization/en/error_recovery.json index 392c9c694c3..b46e276c48b 100644 --- a/app/src/assets/localization/en/error_recovery.json +++ b/app/src/assets/localization/en/error_recovery.json @@ -43,7 +43,7 @@ "ignore_similar_errors_later_in_run": "Ignore similar errors later in the run?", "inspect_the_robot": "First, inspect the robot to ensure it's prepared to continue the run from the next step.Then, close the robot door before proceeding.", "labware_released_from_current_height": "The labware will be released from its current height.", - "launch_recovery_mode": "Launch Recovery Mode", + "launch_recovery_mode": "Launch recovery mode", "manually_fill_liquid_in_well": "Manually fill liquid in well {{well}}", "manually_fill_well_and_skip": "Manually fill well and skip to next step", "manually_move_lw_and_skip": "Manually move labware and skip to next step", @@ -60,7 +60,7 @@ "proceed_to_cancel": "Proceed to cancel", "proceed_to_tip_selection": "Proceed to tip selection", "recovery_action_failed": "{{action}} failed", - "recovery_mode": "Recovery Mode", + "recovery_mode": "Recovery mode", "recovery_mode_explanation": "Recovery Mode provides you with guided and manual controls for handling errors at runtime.
You can make changes to ensure the step in progress when the error occurred can be completed or choose to cancel the protocol. When changes are made and no subsequent errors are detected, the method completes. Depending on the conditions that caused the error, you will only be provided with appropriate options.", "release": "Release", "release_labware_from_gripper": "Release labware from gripper", diff --git a/app/src/assets/localization/en/quick_transfer.json b/app/src/assets/localization/en/quick_transfer.json index c986da098c1..2ebdb5699b8 100644 --- a/app/src/assets/localization/en/quick_transfer.json +++ b/app/src/assets/localization/en/quick_transfer.json @@ -5,7 +5,7 @@ "advanced_setting_disabled": "Advanced setting disabled for this transfer", "advanced_settings": "Advanced settings", "air_gap": "Air gap", - "air_gap_before_aspirating": "Air gap before aspirating", + "air_gap_after_aspirating": "Air gap after aspirating", "air_gap_before_dispensing": "Air gap before dispensing", "air_gap_capacity_error": "The tip is too full to add an air gap.", "air_gap_value": "{{volume}} µL", diff --git a/app/src/assets/localization/en/shared.json b/app/src/assets/localization/en/shared.json index 0b580a612e8..f4e542e761b 100644 --- a/app/src/assets/localization/en/shared.json +++ b/app/src/assets/localization/en/shared.json @@ -11,6 +11,7 @@ "change_robot": "Change robot", "clear_data": "clear data", "close": "close", + "closed": "closed", "close_robot_door": "Close the robot door before starting the run.", "confirm": "Confirm", "confirm_placement": "Confirm placement", diff --git a/app/src/assets/localization/zh/quick_transfer.json b/app/src/assets/localization/zh/quick_transfer.json index f57d7315651..5ecce11bb9c 100644 --- a/app/src/assets/localization/zh/quick_transfer.json +++ b/app/src/assets/localization/zh/quick_transfer.json @@ -4,7 +4,6 @@ "add_or_remove": "添加或移除", "advanced_setting_disabled": "此移液的高级设置已禁用", "advanced_settings": "高级设置", - "air_gap_before_aspirating": "在吸液前设置空气间隙", "air_gap_before_dispensing": "在分液前设置空气间隙", "air_gap_capacity_error": "移液器空间已满，无法添加空气间隙。", "air_gap_value": "{{volume}} µL", diff --git a/app/src/local-resources/modules/utils/getModuleImage.ts b/app/src/local-resources/modules/utils/getModuleImage.ts index f5db2094dcc..b68d5aa8562 100644 --- a/app/src/local-resources/modules/utils/getModuleImage.ts +++ b/app/src/local-resources/modules/utils/getModuleImage.ts @@ -8,6 +8,7 @@ import thermoModuleGen1HighRes from '/app/assets/images/modules/thermocyclerModu import heaterShakerModuleHighRes from '/app/assets/images/modules/heaterShakerModuleV1@3x.png' import thermoModuleGen2 from '/app/assets/images/thermocycler_gen_2_closed.png' import magneticBlockGen1 from '/app/assets/images/magnetic_block_gen_1.png' +import magneticBlockGen1HighRes from '/app/assets/images/magnetic_block_gen_1@3x.png' import absorbanceReader from '/app/assets/images/opentrons_plate_reader.png' import type { ModuleModel } from '@opentrons/shared-data' @@ -30,7 +31,7 @@ export function getModuleImage( case 'thermocyclerModuleV2': return thermoModuleGen2 case 'magneticBlockV1': - return magneticBlockGen1 + return highRes ? magneticBlockGen1HighRes : magneticBlockGen1 case 'absorbanceReaderV1': return absorbanceReader default: diff --git a/app/src/molecules/InterventionModal/index.tsx b/app/src/molecules/InterventionModal/index.tsx index 625b478a8e9..679d3b3d1f8 100644 --- a/app/src/molecules/InterventionModal/index.tsx +++ b/app/src/molecules/InterventionModal/index.tsx @@ -23,7 +23,6 @@ import { import { getIsOnDevice } from '/app/redux/config' -import type { IconName } from '@opentrons/components' import { ModalContentOneColSimpleButtons } from './ModalContentOneColSimpleButtons' import { TwoColumn } from './TwoColumn' import { OneColumn } from './OneColumn' @@ -32,6 +31,10 @@ import { ModalContentMixed } from './ModalContentMixed' import { DescriptionContent } from './DescriptionContent' import { DeckMapContent } from './DeckMapContent' import { CategorizedStepContent } from './CategorizedStepContent' + +import type { FlattenSimpleInterpolation } from 'styled-components' +import type { IconName } from '@opentrons/components' + export { ModalContentOneColSimpleButtons, TwoColumn, @@ -122,6 +125,8 @@ export interface InterventionModalProps { type?: ModalType /** optional icon name */ iconName?: IconName | null | undefined + /* Optional icon size override. */ + iconSize?: string /** modal contents */ children: React.ReactNode } @@ -133,6 +138,7 @@ export function InterventionModal({ iconName, iconHeading, children, + iconSize, }: InterventionModalProps): JSX.Element { const modalType = type ?? 'intervention-required' const headerColor = @@ -166,7 +172,7 @@ export function InterventionModal({ {titleHeading} {iconName != null ? ( - + ) : null} {iconHeading != null ? iconHeading : null} @@ -178,15 +184,15 @@ export function InterventionModal({ ) } -const ICON_STYLE = css` - width: ${SPACING.spacing16}; - height: ${SPACING.spacing16}; +const buildIconStyle = ( + iconSize: string | undefined +): FlattenSimpleInterpolation => css` + width: ${iconSize ?? SPACING.spacing16}; + height: ${iconSize ?? SPACING.spacing16}; margin: ${SPACING.spacing4}; cursor: ${CURSOR_POINTER}; @media (${RESPONSIVENESS.touchscreenMediaQuerySpecs}) { - width: ${SPACING.spacing32}; - height: ${SPACING.spacing32}; margin: ${SPACING.spacing12}; } ` diff --git a/app/src/molecules/WizardRequiredEquipmentList/index.tsx b/app/src/molecules/WizardRequiredEquipmentList/index.tsx index 69c5cd02893..f6f7457a71a 100644 --- a/app/src/molecules/WizardRequiredEquipmentList/index.tsx +++ b/app/src/molecules/WizardRequiredEquipmentList/index.tsx @@ -155,7 +155,7 @@ function RequiredEquipmentCard(props: RequiredEquipmentCardProps): JSX.Element { ) : null} diff --git a/app/src/organisms/Desktop/Devices/HistoricalProtocolRun.tsx b/app/src/organisms/Desktop/Devices/HistoricalProtocolRun.tsx index 1ec488cfc45..b9e05314f80 100644 --- a/app/src/organisms/Desktop/Devices/HistoricalProtocolRun.tsx +++ b/app/src/organisms/Desktop/Devices/HistoricalProtocolRun.tsx @@ -41,12 +41,15 @@ export function HistoricalProtocolRun( const { t } = useTranslation('run_details') const { run, protocolName, robotIsBusy, robotName, protocolKey } = props const [drawerOpen, setDrawerOpen] = useState(false) - const countRunDataFiles = + let countRunDataFiles = 'runTimeParameters' in run ? run?.runTimeParameters.filter( parameter => parameter.type === 'csv_file' ).length : 0 + if ('outputFileIds' in run) { + countRunDataFiles += run.outputFileIds.length + } const runStatus = run.status const runDisplayName = formatTimestamp(run.createdAt) let duration = EMPTY_TIMESTAMP diff --git a/app/src/organisms/Desktop/Devices/ProtocolRun/ProtocolRunHeader/RunHeaderContent/ActionButton/hooks/useActionButtonProperties.ts b/app/src/organisms/Desktop/Devices/ProtocolRun/ProtocolRunHeader/RunHeaderContent/ActionButton/hooks/useActionButtonProperties.ts index 4c5486eb6e7..f9ea87080ca 100644 --- a/app/src/organisms/Desktop/Devices/ProtocolRun/ProtocolRunHeader/RunHeaderContent/ActionButton/hooks/useActionButtonProperties.ts +++ b/app/src/organisms/Desktop/Devices/ProtocolRun/ProtocolRunHeader/RunHeaderContent/ActionButton/hooks/useActionButtonProperties.ts @@ -132,6 +132,7 @@ export function useActionButtonProperties({ handleButtonClick = () => { isResetRunLoadingRef.current = true reset() + runHeaderModalContainerUtils.dropTipUtils.resetTipStatus() trackEvent({ name: ANALYTICS_PROTOCOL_PROCEED_TO_RUN, properties: { sourceLocation: 'RunRecordDetail', robotSerialNumber }, diff --git a/app/src/organisms/Desktop/Devices/ProtocolRun/ProtocolRunHeader/RunHeaderModalContainer/hooks/useRunHeaderDropTip.ts b/app/src/organisms/Desktop/Devices/ProtocolRun/ProtocolRunHeader/RunHeaderModalContainer/hooks/useRunHeaderDropTip.ts index 687b0d404c5..2045312af4b 100644 --- a/app/src/organisms/Desktop/Devices/ProtocolRun/ProtocolRunHeader/RunHeaderModalContainer/hooks/useRunHeaderDropTip.ts +++ b/app/src/organisms/Desktop/Devices/ProtocolRun/ProtocolRunHeader/RunHeaderModalContainer/hooks/useRunHeaderDropTip.ts @@ -17,7 +17,10 @@ import { useTipAttachmentStatus } from '/app/resources/instruments' import type { RobotType } from '@opentrons/shared-data' import type { Run, RunStatus } from '@opentrons/api-client' import type { PipetteWithTip } from '/app/resources/instruments' -import type { DropTipWizardFlowsProps } from '/app/organisms/DropTipWizardFlows' +import type { + DropTipWizardFlowsProps, + TipAttachmentStatusResult, +} from '/app/organisms/DropTipWizardFlows' import type { UseProtocolDropTipModalResult } from '../modals' import type { PipetteDetails } from '/app/resources/maintenance_runs' @@ -35,6 +38,7 @@ export interface UseRunHeaderDropTipParams { export interface UseRunHeaderDropTipResult { dropTipModalUtils: UseProtocolDropTipModalResult dropTipWizardUtils: RunHeaderDropTipWizProps + resetTipStatus: TipAttachmentStatusResult['resetTipStatus'] } // Handles all the tip related logic during a protocol run on the desktop app. @@ -104,11 +108,9 @@ export function useRunHeaderDropTip({ { includeFixitCommands: false, pageLength: 1, - cursor: null, }, { enabled: isTerminalRunStatus(runStatus) } ) - // Manage tip checking useEffect(() => { // If a user begins a new run without navigating away from the run page, reset tip status. @@ -120,7 +122,9 @@ export function useRunHeaderDropTip({ // have to do it here if done during Error Recovery. else if ( runSummaryNoFixit != null && - !lastRunCommandPromptedErrorRecovery(runSummaryNoFixit) + runSummaryNoFixit.length > 0 && + !lastRunCommandPromptedErrorRecovery(runSummaryNoFixit) && + isTerminalRunStatus(runStatus) ) { void determineTipStatus() } @@ -143,7 +147,11 @@ export function useRunHeaderDropTip({ } }, [runStatus, isRunCurrent, enteredER, initialPipettesWithTipsCount]) - return { dropTipModalUtils, dropTipWizardUtils: buildDTWizUtils() } + return { + dropTipModalUtils, + dropTipWizardUtils: buildDTWizUtils(), + resetTipStatus, + } } // TODO(jh, 09-12-24): Consolidate this with the same utility that exists elsewhere. diff --git a/app/src/organisms/Desktop/Devices/RobotSettings/AdvancedTab/__tests__/EnableErrorRecoveryMode.test.tsx b/app/src/organisms/Desktop/Devices/RobotSettings/AdvancedTab/__tests__/EnableErrorRecoveryMode.test.tsx index a2d4824951f..9406e38f768 100644 --- a/app/src/organisms/Desktop/Devices/RobotSettings/AdvancedTab/__tests__/EnableErrorRecoveryMode.test.tsx +++ b/app/src/organisms/Desktop/Devices/RobotSettings/AdvancedTab/__tests__/EnableErrorRecoveryMode.test.tsx @@ -32,7 +32,7 @@ describe('EnableErrorRecoveryMode', () => { it('should render text and toggle button', () => { render(props) - screen.getByText('Error Recovery Mode') + screen.getByText('Recovery mode') screen.getByText('Pause on protocol errors instead of canceling the run.') expect( screen.getByLabelText('enable_error_recovery_mode') diff --git a/app/src/organisms/Desktop/Devices/hooks/useDownloadRunLog.ts b/app/src/organisms/Desktop/Devices/hooks/useDownloadRunLog.ts index 61acbfb12c4..6e07d5bbfab 100644 --- a/app/src/organisms/Desktop/Devices/hooks/useDownloadRunLog.ts +++ b/app/src/organisms/Desktop/Devices/hooks/useDownloadRunLog.ts @@ -27,7 +27,6 @@ export function useDownloadRunLog( if (host == null) return // first getCommands to get total length of commands getCommands(host, runId, { - cursor: null, pageLength: 0, includeFixitCommands: true, }) diff --git a/app/src/organisms/Desktop/RunProgressMeter/__tests__/RunProgressMeter.test.tsx b/app/src/organisms/Desktop/RunProgressMeter/__tests__/RunProgressMeter.test.tsx index 3618b0ce586..928bd2572a9 100644 --- a/app/src/organisms/Desktop/RunProgressMeter/__tests__/RunProgressMeter.test.tsx +++ b/app/src/organisms/Desktop/RunProgressMeter/__tests__/RunProgressMeter.test.tsx @@ -77,7 +77,6 @@ describe('RunProgressMeter', () => { .thenReturn(null) when(useNotifyAllCommandsQuery) .calledWith(NON_DETERMINISTIC_RUN_ID, { - cursor: null, pageLength: 1, }) .thenReturn(mockUseAllCommandsResponseNonDeterministic) diff --git a/app/src/organisms/Desktop/RunProgressMeter/index.tsx b/app/src/organisms/Desktop/RunProgressMeter/index.tsx index 481947e1707..fb158f5d686 100644 --- a/app/src/organisms/Desktop/RunProgressMeter/index.tsx +++ b/app/src/organisms/Desktop/RunProgressMeter/index.tsx @@ -67,7 +67,6 @@ export function RunProgressMeter(props: RunProgressMeterProps): JSX.Element { const runData = runRecord?.data ?? null const { data: mostRecentCommandData } = useNotifyAllCommandsQuery(runId, { - cursor: null, pageLength: 1, }) // This lastRunCommand also includes "fixit" commands. diff --git a/app/src/organisms/DropTipWizardFlows/DropTipWizardHeader.tsx b/app/src/organisms/DropTipWizardFlows/DropTipWizardHeader.tsx index e883da67d2b..55363426104 100644 --- a/app/src/organisms/DropTipWizardFlows/DropTipWizardHeader.tsx +++ b/app/src/organisms/DropTipWizardFlows/DropTipWizardHeader.tsx @@ -1,11 +1,9 @@ -import { useState, useEffect } from 'react' import { useTranslation } from 'react-i18next' -import { BEFORE_BEGINNING, BLOWOUT_SUCCESS, DT_ROUTES } from './constants' import { WizardHeader } from '/app/molecules/WizardHeader' import type { DropTipWizardProps } from './DropTipWizard' -import type { DropTipFlowsRoute, DropTipFlowsStep, ErrorDetails } from './types' +import type { ErrorDetails } from './types' type DropTipWizardHeaderProps = DropTipWizardProps & { isExitInitiated: boolean @@ -16,9 +14,6 @@ type DropTipWizardHeaderProps = DropTipWizardProps & { export function DropTipWizardHeader({ confirmExit, - currentStep, - currentRoute, - currentStepIdx, isExitInitiated, isFinalWizardStep, errorDetails, @@ -36,73 +31,14 @@ export function DropTipWizardHeader({ handleCleanUpAndClose, }) - const { totalSteps, currentStepNumber } = useSeenBlowoutSuccess({ - currentStep, - currentRoute, - currentStepIdx, - }) - return ( ) } -interface UseSeenBlowoutSuccessProps { - currentStep: DropTipFlowsStep - currentRoute: DropTipFlowsRoute - currentStepIdx: number -} - -interface UseSeenBlowoutSuccessResult { - currentStepNumber: number | null - totalSteps: number | null -} - -// Calculate the props used for determining step count based on the route. Because blowout and drop tip are separate routes, -// there's a need for state to track whether we've seen blowout, so the step counter is accurate when the drop tip route is active. -export function useSeenBlowoutSuccess({ - currentStep, - currentRoute, - currentStepIdx, -}: UseSeenBlowoutSuccessProps): UseSeenBlowoutSuccessResult { - const [hasSeenBlowoutSuccess, setHasSeenBlowoutSuccess] = useState(false) - - useEffect(() => { - if (currentStep === BLOWOUT_SUCCESS) { - setHasSeenBlowoutSuccess(true) - } else if (currentStep === BEFORE_BEGINNING) { - setHasSeenBlowoutSuccess(false) - } - }, [currentStep]) - - const shouldRenderStepCounter = currentRoute !== DT_ROUTES.BEFORE_BEGINNING - - let totalSteps: null | number - if (!shouldRenderStepCounter) { - totalSteps = null - } else if (currentRoute === DT_ROUTES.BLOWOUT || hasSeenBlowoutSuccess) { - totalSteps = DT_ROUTES.BLOWOUT.length + DT_ROUTES.DROP_TIP.length - } else { - totalSteps = currentRoute.length - } - - let currentStepNumber: null | number - if (!shouldRenderStepCounter) { - currentStepNumber = null - } else if (hasSeenBlowoutSuccess && currentRoute === DT_ROUTES.DROP_TIP) { - currentStepNumber = DT_ROUTES.BLOWOUT.length + currentStepIdx + 1 - } else { - currentStepNumber = currentStepIdx + 1 - } - - return { currentStepNumber, totalSteps } -} - export interface UseWizardExitHeaderProps { isFinalStep: boolean hasInitiatedExit: boolean diff --git a/app/src/organisms/DropTipWizardFlows/__tests__/DropTipWizardHeader.test.tsx b/app/src/organisms/DropTipWizardFlows/__tests__/DropTipWizardHeader.test.tsx index 306b3eff6d1..c5720adf4ab 100644 --- a/app/src/organisms/DropTipWizardFlows/__tests__/DropTipWizardHeader.test.tsx +++ b/app/src/organisms/DropTipWizardFlows/__tests__/DropTipWizardHeader.test.tsx @@ -1,16 +1,14 @@ import type * as React from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' -import { renderHook, screen } from '@testing-library/react' +import { screen } from '@testing-library/react' import { renderWithProviders } from '/app/__testing-utils__' import { i18n } from '/app/i18n' import { mockDropTipWizardContainerProps } from '../__fixtures__' import { useWizardExitHeader, - useSeenBlowoutSuccess, DropTipWizardHeader, } from '../DropTipWizardHeader' -import { DT_ROUTES } from '../constants' import type { Mock } from 'vitest' import type { UseWizardExitHeaderProps } from '../DropTipWizardHeader' @@ -31,22 +29,6 @@ describe('DropTipWizardHeader', () => { it('renders appropriate copy and onClick behavior', () => { render(props) screen.getByText('Drop tips') - screen.getByText('Step 1 / 5') - }) -}) - -describe('useSeenBlowoutSuccess', () => { - it('should not render step counter when currentRoute is BEFORE_BEGINNING', () => { - const { result } = renderHook(() => - useSeenBlowoutSuccess({ - currentStep: 'SOME_STEP' as any, - currentRoute: DT_ROUTES.BEFORE_BEGINNING, - currentStepIdx: 0, - }) - ) - - expect(result.current.totalSteps).toBe(null) - expect(result.current.currentStepNumber).toBe(null) }) }) diff --git a/app/src/organisms/DropTipWizardFlows/hooks/__tests__/errors.test.tsx b/app/src/organisms/DropTipWizardFlows/hooks/__tests__/errors.test.tsx index 08caf96c946..476513706f9 100644 --- a/app/src/organisms/DropTipWizardFlows/hooks/__tests__/errors.test.tsx +++ b/app/src/organisms/DropTipWizardFlows/hooks/__tests__/errors.test.tsx @@ -29,9 +29,9 @@ describe('useDropTipCommandErrors', () => { act(() => { result.current({ - runCommandError: { - errorType: DROP_TIP_SPECIAL_ERROR_TYPES.MUST_HOME_ERROR, - } as any, + type: DROP_TIP_SPECIAL_ERROR_TYPES.MUST_HOME_ERROR, + message: 'remove_the_tips_manually', + header: 'cant_safely_drop_tips', }) }) diff --git a/app/src/organisms/DropTipWizardFlows/hooks/errors.tsx b/app/src/organisms/DropTipWizardFlows/hooks/errors.tsx index c8df4d239a9..1f7d6cf09ff 100644 --- a/app/src/organisms/DropTipWizardFlows/hooks/errors.tsx +++ b/app/src/organisms/DropTipWizardFlows/hooks/errors.tsx @@ -9,8 +9,7 @@ import type { RunCommandError } from '@opentrons/shared-data' import type { ErrorDetails } from '../types' export interface SetRobotErrorDetailsParams { - runCommandError?: RunCommandError - message?: string + message: string | null header?: string type?: RunCommandError['errorType'] } @@ -23,16 +22,8 @@ export function useDropTipCommandErrors( ): (cbProps: SetRobotErrorDetailsParams) => void { const { t } = useTranslation('drop_tip_wizard') - return ({ - runCommandError, - message, - header, - type, - }: SetRobotErrorDetailsParams) => { - if ( - runCommandError?.errorType === - DROP_TIP_SPECIAL_ERROR_TYPES.MUST_HOME_ERROR - ) { + return ({ message, header, type }: SetRobotErrorDetailsParams) => { + if (type === DROP_TIP_SPECIAL_ERROR_TYPES.MUST_HOME_ERROR) { const headerText = t('cant_safely_drop_tips') const messageText = t('remove_the_tips_manually') diff --git a/app/src/organisms/DropTipWizardFlows/hooks/useDropTipCommands.ts b/app/src/organisms/DropTipWizardFlows/hooks/useDropTipCommands.ts index 493b4a77bff..c3162a48f07 100644 --- a/app/src/organisms/DropTipWizardFlows/hooks/useDropTipCommands.ts +++ b/app/src/organisms/DropTipWizardFlows/hooks/useDropTipCommands.ts @@ -2,13 +2,18 @@ import { useState, useEffect } from 'react' import { useDeleteMaintenanceRunMutation } from '@opentrons/react-api-client' -import { DT_ROUTES, MANAGED_PIPETTE_ID } from '../constants' +import { + DROP_TIP_SPECIAL_ERROR_TYPES, + DT_ROUTES, + MANAGED_PIPETTE_ID, +} from '../constants' import { getAddressableAreaFromConfig } from '../utils' import { useNotifyDeckConfigurationQuery } from '/app/resources/deck_configuration' import type { CreateCommand, AddressableAreaName, PipetteModelSpecs, + RunCommandError, } from '@opentrons/shared-data' import { FLEX_ROBOT_TYPE } from '@opentrons/shared-data' import type { CommandData, PipetteData } from '@opentrons/api-client' @@ -129,17 +134,25 @@ export function useDropTipCommands({ issuedCommandsType ) - return chainRunCommands( - isFlex - ? [ - ENGAGE_AXES, - UPDATE_ESTIMATORS_EXCEPT_PLUNGERS, - Z_HOME, - moveToAACommand, - ] - : [Z_HOME, moveToAACommand], - false - ) + if (isFlex) { + return chainRunCommands( + [ENGAGE_AXES, UPDATE_ESTIMATORS_EXCEPT_PLUNGERS], + false + ) + .catch(error => { + // If one of the engage/estimator commands fails, we can safely assume it's because the position is + // unknown, so show the special error modal. + throw { + ...error, + errorType: DROP_TIP_SPECIAL_ERROR_TYPES.MUST_HOME_ERROR, + } + }) + .then(() => { + return chainRunCommands([Z_HOME, moveToAACommand], false) + }) + } else { + return chainRunCommands([Z_HOME, moveToAACommand], false) + } }) .then((commandData: CommandData[]) => { const error = commandData[0].data.error @@ -149,12 +162,12 @@ export function useDropTipCommands({ } resolve() }) - .catch(error => { + .catch((error: RunCommandError) => { if (fixitCommandTypeUtils != null && issuedCommandsType === 'fixit') { fixitCommandTypeUtils.errorOverrides.generalFailure() } else { setErrorDetails({ - runCommandError: error, + type: error.errorType ?? null, message: error.detail ? `Error moving to position: ${error.detail}` : 'Error moving to position: invalid addressable area.', @@ -224,51 +237,70 @@ export function useDropTipCommands({ proceed: () => void ): Promise => { return new Promise((resolve, reject) => { - chainRunCommands( - currentRoute === DT_ROUTES.BLOWOUT - ? buildBlowoutCommands(instrumentModelSpecs, isFlex, pipetteId) - : buildDropTipInPlaceCommand(isFlex, pipetteId), - false - ) - .then((commandData: CommandData[]) => { - const error = commandData[0].data.error - if (error != null) { - if ( - fixitCommandTypeUtils != null && - issuedCommandsType === 'fixit' - ) { - if (currentRoute === DT_ROUTES.BLOWOUT) { - fixitCommandTypeUtils.errorOverrides.blowoutFailed() - } else { - fixitCommandTypeUtils.errorOverrides.tipDropFailed() - } - } - - setErrorDetails({ - runCommandError: error, - message: `Error moving to position: ${error.detail}`, - }) - } else { - proceed() - resolve() - } - }) - .catch((error: Error) => { - if (fixitCommandTypeUtils != null && issuedCommandsType === 'fixit') { - if (currentRoute === DT_ROUTES.BLOWOUT) { - fixitCommandTypeUtils.errorOverrides.blowoutFailed() - } else { - fixitCommandTypeUtils.errorOverrides.tipDropFailed() - } - } + const isBlowoutRoute = currentRoute === DT_ROUTES.BLOWOUT + + const handleError = (error: RunCommandError | Error): void => { + if (fixitCommandTypeUtils != null && issuedCommandsType === 'fixit') { + isBlowoutRoute + ? fixitCommandTypeUtils.errorOverrides.blowoutFailed() + : fixitCommandTypeUtils.errorOverrides.tipDropFailed() + } else { + const operation = isBlowoutRoute ? 'blowout' : 'drop tip' + const type = 'errorType' in error ? error.errorType : undefined + const messageDetail = + 'message' in error ? error.message : error.detail setErrorDetails({ - message: `Error issuing ${ - currentRoute === DT_ROUTES.BLOWOUT ? 'blowout' : 'drop tip' - } command: ${error.message}`, + type, + message: + messageDetail != null + ? `Error during ${operation}: ${messageDetail}` + : null, }) - resolve() + } + reject(error) + } + + // Throw any errors in the response body if any. + const handleSuccess = (commandData: CommandData[]): void => { + const error = commandData[0].data.error + if (error != null) { + // eslint-disable-next-line @typescript-eslint/no-throw-literal + throw error + } + proceed() + resolve() + } + + // For Flex, we need extra preparation steps + const prepareFlexBlowout = (): Promise => { + return chainRunCommands( + [ENGAGE_AXES, UPDATE_PLUNGER_ESTIMATORS], + false + ).catch(error => { + throw { + ...error, + errorType: DROP_TIP_SPECIAL_ERROR_TYPES.MUST_HOME_ERROR, + } }) + } + + const executeCommands = (): Promise => { + const commands = isBlowoutRoute + ? buildBlowoutCommands(instrumentModelSpecs, isFlex, pipetteId) + : buildDropTipInPlaceCommand(isFlex, pipetteId) + + return chainRunCommands(commands, false) + } + + if (isBlowoutRoute && isFlex) { + prepareFlexBlowout() + .then(executeCommands) + .then(handleSuccess) + .catch(handleError) + } else { + executeCommands().then(handleSuccess).catch(handleError) + } }) } @@ -356,13 +388,10 @@ const buildBlowoutCommands = ( ): CreateCommand[] => isFlex ? [ - ENGAGE_AXES, - UPDATE_PLUNGER_ESTIMATORS, { commandType: 'unsafe/blowOutInPlace', params: { pipetteId: pipetteId ?? MANAGED_PIPETTE_ID, - flowRate: Math.min( specs.defaultBlowOutFlowRate.value, MAXIMUM_BLOWOUT_FLOW_RATE_UL_PER_S @@ -376,7 +405,6 @@ const buildBlowoutCommands = ( commandType: 'blowOutInPlace', params: { pipetteId: pipetteId ?? MANAGED_PIPETTE_ID, - flowRate: specs.defaultBlowOutFlowRate.value, }, }, diff --git a/app/src/organisms/DropTipWizardFlows/steps/BeforeBeginning.tsx b/app/src/organisms/DropTipWizardFlows/steps/BeforeBeginning.tsx index bbcb908ea24..82be17832b4 100644 --- a/app/src/organisms/DropTipWizardFlows/steps/BeforeBeginning.tsx +++ b/app/src/organisms/DropTipWizardFlows/steps/BeforeBeginning.tsx @@ -235,6 +235,10 @@ const CONTAINER_STYLE = css` display: ${DISPLAY_FLEX}; flex-direction: ${DIRECTION_COLUMN}; grid-gap: ${SPACING.spacing16}; + + @media ${RESPONSIVENESS.touchscreenMediaQuerySpecs} { + grid-gap: ${SPACING.spacing8}; + } ` const ODD_MEDIUM_BUTTON_STYLE = css` diff --git a/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx b/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx index 795b1517ee8..2c6f047f80d 100644 --- a/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx @@ -107,10 +107,7 @@ export function ErrorRecoveryComponent( const buildTitleHeading = (): JSX.Element => { const titleText = hasLaunchedRecovery ? t('recovery_mode') : t('cancel_run') return ( - + {titleText} ) diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryTakeover.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryTakeover.tsx index eccd4e972a7..4a28ac24b0c 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryTakeover.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryTakeover.tsx @@ -120,9 +120,17 @@ export function RecoveryTakeoverDesktop({ }: RecoveryTakeoverProps): JSX.Element { const { t } = useTranslation('error_recovery') + const buildTitleHeadingDesktop = (): JSX.Element => { + return ( + + {t('error_on_robot', { robot: robotName })} + + ) + } + return ( diff --git a/app/src/organisms/ErrorRecoveryFlows/__tests__/RecoverySplash.test.tsx b/app/src/organisms/ErrorRecoveryFlows/__tests__/RecoverySplash.test.tsx index 7446ea7464c..901ab22e158 100644 --- a/app/src/organisms/ErrorRecoveryFlows/__tests__/RecoverySplash.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/__tests__/RecoverySplash.test.tsx @@ -136,7 +136,7 @@ describe('RecoverySplash', () => { render(props) const primaryBtn = screen.getByRole('button', { - name: 'Launch Recovery Mode', + name: 'Launch recovery mode', }) const secondaryBtn = screen.getByRole('button', { name: 'Cancel run' }) @@ -183,7 +183,7 @@ describe('RecoverySplash', () => { render(props) - clickButtonLabeled('Launch Recovery Mode') + clickButtonLabeled('Launch recovery mode') expect(mockMakeToast).toHaveBeenCalled() }) diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useCurrentlyRecoveringFrom.test.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useCurrentlyRecoveringFrom.test.ts index 4b177972851..cc27994d671 100644 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useCurrentlyRecoveringFrom.test.ts +++ b/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useCurrentlyRecoveringFrom.test.ts @@ -42,9 +42,11 @@ describe('useCurrentlyRecoveringFrom', () => { }, }, }, + isFetching: false, } as any) vi.mocked(useCommandQuery).mockReturnValue({ data: { data: 'mockCommandDetails' }, + isFetching: false, } as any) const { result } = renderHook(() => @@ -53,7 +55,7 @@ describe('useCurrentlyRecoveringFrom', () => { expect(vi.mocked(useNotifyAllCommandsQuery)).toHaveBeenCalledWith( MOCK_RUN_ID, - { cursor: null, pageLength: 0 }, + { pageLength: 0 }, { enabled: false, refetchInterval: 5000 } ) expect(vi.mocked(useCommandQuery)).toHaveBeenCalledWith( @@ -69,8 +71,11 @@ describe('useCurrentlyRecoveringFrom', () => { data: { links: {}, }, + isFetching: false, + } as any) + vi.mocked(useCommandQuery).mockReturnValue({ + isFetching: false, } as any) - vi.mocked(useCommandQuery).mockReturnValue({} as any) const { result } = renderHook(() => useCurrentlyRecoveringFrom(MOCK_RUN_ID, RUN_STATUS_AWAITING_RECOVERY) @@ -94,9 +99,11 @@ describe('useCurrentlyRecoveringFrom', () => { }, }, }, + isFetching: false, } as any) vi.mocked(useCommandQuery).mockReturnValue({ data: { data: 'mockCommandDetails' }, + isFetching: false, } as any) const { result } = renderHook(() => @@ -111,6 +118,91 @@ describe('useCurrentlyRecoveringFrom', () => { expect(result.current).toStrictEqual('mockCommandDetails') }) + it('returns null if all commands query is still fetching', () => { + vi.mocked(useNotifyAllCommandsQuery).mockReturnValue({ + data: { + links: { + currentlyRecoveringFrom: { + meta: { + runId: MOCK_RUN_ID, + commandId: MOCK_COMMAND_ID, + }, + }, + }, + }, + isFetching: true, + } as any) + vi.mocked(useCommandQuery).mockReturnValue({ + data: { data: 'mockCommandDetails' }, + isFetching: false, + } as any) + + const { result } = renderHook(() => + useCurrentlyRecoveringFrom(MOCK_RUN_ID, RUN_STATUS_AWAITING_RECOVERY) + ) + + expect(result.current).toStrictEqual(null) + }) + + it('returns null if command query is still fetching', () => { + vi.mocked(useNotifyAllCommandsQuery).mockReturnValue({ + data: { + links: { + currentlyRecoveringFrom: { + meta: { + runId: MOCK_RUN_ID, + commandId: MOCK_COMMAND_ID, + }, + }, + }, + }, + isFetching: false, + } as any) + vi.mocked(useCommandQuery).mockReturnValue({ + data: { data: 'mockCommandDetails' }, + isFetching: true, + } as any) + + const { result } = renderHook(() => + useCurrentlyRecoveringFrom(MOCK_RUN_ID, RUN_STATUS_AWAITING_RECOVERY) + ) + + expect(result.current).toStrictEqual(null) + }) + + it('resets isReadyToShow when run exits recovery mode', () => { + const { rerender, result } = renderHook( + ({ status }) => useCurrentlyRecoveringFrom(MOCK_RUN_ID, status), + { initialProps: { status: RUN_STATUS_AWAITING_RECOVERY } } + ) + + vi.mocked(useNotifyAllCommandsQuery).mockReturnValue({ + data: { + links: { + currentlyRecoveringFrom: { + meta: { + runId: MOCK_RUN_ID, + commandId: MOCK_COMMAND_ID, + }, + }, + }, + }, + isFetching: false, + } as any) + vi.mocked(useCommandQuery).mockReturnValue({ + data: { data: 'mockCommandDetails' }, + isFetching: false, + } as any) + + rerender({ status: RUN_STATUS_AWAITING_RECOVERY }) + + expect(result.current).toStrictEqual('mockCommandDetails') + + rerender({ status: RUN_STATUS_IDLE } as any) + + expect(result.current).toStrictEqual(null) + }) + it('calls invalidateQueries when the run enters recovery mode', () => { renderHook(() => useCurrentlyRecoveringFrom(MOCK_RUN_ID, RUN_STATUS_AWAITING_RECOVERY) diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts index 21307b8e4e8..ce333c1fb39 100644 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts +++ b/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryCommands.test.ts @@ -4,11 +4,13 @@ import { renderHook, act } from '@testing-library/react' import { useResumeRunFromRecoveryMutation, useStopRunMutation, - useUpdateErrorRecoveryPolicy, useResumeRunFromRecoveryAssumingFalsePositiveMutation, } from '@opentrons/react-api-client' -import { useChainRunCommands } from '/app/resources/runs' +import { + useChainRunCommands, + useUpdateRecoveryPolicyWithStrategy, +} from '/app/resources/runs' import { useRecoveryCommands, HOME_PIPETTE_Z_AXES, @@ -80,9 +82,9 @@ describe('useRecoveryCommands', () => { vi.mocked(useChainRunCommands).mockReturnValue({ chainRunCommands: mockChainRunCommands, } as any) - vi.mocked(useUpdateErrorRecoveryPolicy).mockReturnValue({ - mutateAsync: mockUpdateErrorRecoveryPolicy, - } as any) + vi.mocked(useUpdateRecoveryPolicyWithStrategy).mockReturnValue( + mockUpdateErrorRecoveryPolicy as any + ) vi.mocked( useResumeRunFromRecoveryAssumingFalsePositiveMutation ).mockReturnValue({ @@ -361,7 +363,8 @@ describe('useRecoveryCommands', () => { ) expect(mockUpdateErrorRecoveryPolicy).toHaveBeenCalledWith( - expectedPolicyRules + expectedPolicyRules, + 'append' ) }) diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryToasts.test.tsx b/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryToasts.test.tsx index c9874eb5532..085551f42e7 100644 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryToasts.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useRecoveryToasts.test.tsx @@ -94,13 +94,13 @@ describe('useRecoveryToasts', () => { result.current.makeSuccessToast() expect(mockMakeToast).toHaveBeenCalledWith( - 'Retrying step 1 succeeded.', + 'test command', 'success', expect.objectContaining({ closeButton: true, disableTimeout: true, displayType: 'desktop', - heading: expect.any(String), + heading: 'Retrying step 1 succeeded.', }) ) }) @@ -209,8 +209,8 @@ describe('useRecoveryFullCommandText', () => { const { result } = renderHook(() => useRecoveryFullCommandText({ robotType: FLEX_ROBOT_TYPE, - stepNumber: 0, - commandTextData: { commands: [TEST_COMMAND] } as any, + stepNumber: 1, + commandTextData: { commands: [TEST_COMMAND, {}] } as any, allRunDefs: [], }) ) @@ -259,7 +259,7 @@ describe('useRecoveryFullCommandText', () => { const { result } = renderHook(() => useRecoveryFullCommandText({ robotType: FLEX_ROBOT_TYPE, - stepNumber: 0, + stepNumber: 1, commandTextData: { commands: [TC_COMMAND], } as any, @@ -279,9 +279,9 @@ describe('useRecoveryFullCommandText', () => { const { result } = renderHook(() => useRecoveryFullCommandText({ robotType: FLEX_ROBOT_TYPE, - stepNumber: 0, + stepNumber: 1, commandTextData: { - commands: [TC_COMMAND], + commands: [TC_COMMAND, {}], } as any, allRunDefs: [], }) diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/useCurrentlyRecoveringFrom.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/useCurrentlyRecoveringFrom.ts index d0273643b61..38f9e39165a 100644 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/useCurrentlyRecoveringFrom.ts +++ b/app/src/organisms/ErrorRecoveryFlows/hooks/useCurrentlyRecoveringFrom.ts @@ -1,4 +1,4 @@ -import { useEffect } from 'react' +import { useEffect, useState } from 'react' import { useQueryClient } from 'react-query' import { @@ -23,13 +23,15 @@ const VALID_RECOVERY_FETCH_STATUSES = [ ] as Array // Return the `currentlyRecoveringFrom` command returned by the server, if any. -// Otherwise, returns null. +// The command will only be returned after the initial fetches are complete to prevent rendering of stale data. export function useCurrentlyRecoveringFrom( runId: string, runStatus: RunStatus | null ): FailedCommand | null { const queryClient = useQueryClient() const host = useHost() + const [isReadyToShow, setIsReadyToShow] = useState(false) + // There can only be a currentlyRecoveringFrom command when the run is in recovery mode. // In case we're falling back to polling, only enable queries when that is the case. const isRunInRecoveryMode = VALID_RECOVERY_FETCH_STATUSES.includes(runStatus) @@ -38,12 +40,17 @@ export function useCurrentlyRecoveringFrom( useEffect(() => { if (isRunInRecoveryMode) { void queryClient.invalidateQueries([host, 'runs', runId]) + } else { + setIsReadyToShow(false) } }, [isRunInRecoveryMode, host, runId]) - const { data: allCommandsQueryData } = useNotifyAllCommandsQuery( + const { + data: allCommandsQueryData, + isFetching: isAllCommandsFetching, + } = useNotifyAllCommandsQuery( runId, - { cursor: null, pageLength: 0 }, // pageLength 0 because we only care about the links. + { pageLength: 0 }, // pageLength 0 because we only care about the links. { enabled: isRunInRecoveryMode, refetchInterval: ALL_COMMANDS_POLL_MS, @@ -54,7 +61,10 @@ export function useCurrentlyRecoveringFrom( // TODO(mm, 2024-05-21): When the server supports fetching the // currentlyRecoveringFrom command in one step, do that instead of this chained query. - const { data: commandQueryData } = useCommandQuery( + const { + data: commandQueryData, + isFetching: isCommandFetching, + } = useCommandQuery( currentlyRecoveringFromLink?.meta.runId ?? null, currentlyRecoveringFromLink?.meta.commandId ?? null, { @@ -62,5 +72,25 @@ export function useCurrentlyRecoveringFrom( } ) - return isRunInRecoveryMode ? commandQueryData?.data ?? null : null + // Only mark as ready to show when waterfall fetches are complete + useEffect(() => { + if ( + isRunInRecoveryMode && + !isAllCommandsFetching && + !isCommandFetching && + !isReadyToShow + ) { + setIsReadyToShow(true) + } + }, [ + isRunInRecoveryMode, + isAllCommandsFetching, + isCommandFetching, + isReadyToShow, + ]) + + const shouldShowCommand = + isRunInRecoveryMode && isReadyToShow && commandQueryData?.data + + return shouldShowCommand ? commandQueryData.data : null } diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/useERUtils.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/useERUtils.ts index ecf03e4f56b..72e9bb481bc 100644 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/useERUtils.ts +++ b/app/src/organisms/ErrorRecoveryFlows/hooks/useERUtils.ts @@ -1,3 +1,5 @@ +import { useMemo } from 'react' + import { useInstrumentsQuery } from '@opentrons/react-api-client' import { useRouteUpdateActions } from './useRouteUpdateActions' @@ -13,12 +15,12 @@ import { } from '/app/resources/runs' import { useRecoveryOptionCopy } from './useRecoveryOptionCopy' import { useRecoveryActionMutation } from './useRecoveryActionMutation' -import { useRunningStepCounts } from '/app/resources/protocols/hooks' import { useRecoveryToasts } from './useRecoveryToasts' import { useRecoveryAnalytics } from '/app/redux-resources/analytics' import { useShowDoorInfo } from './useShowDoorInfo' import { useCleanupRecoveryState } from './useCleanupRecoveryState' import { useFailedPipetteUtils } from './useFailedPipetteUtils' +import { getRunningStepCountsFrom } from '/app/resources/protocols' import type { LabwareDefinition2, @@ -102,7 +104,14 @@ export function useERUtils({ pageLength: 999, }) - const stepCounts = useRunningStepCounts(runId, runCommands) + const stepCounts = useMemo( + () => + getRunningStepCountsFrom( + protocolAnalysis?.commands ?? [], + failedCommand?.byRunRecord ?? null + ), + [protocolAnalysis != null, failedCommand] + ) const analytics = useRecoveryAnalytics() diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts index 65bd77eed0b..3e4b20225c5 100644 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts +++ b/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryCommands.ts @@ -4,11 +4,13 @@ import head from 'lodash/head' import { useResumeRunFromRecoveryMutation, useStopRunMutation, - useUpdateErrorRecoveryPolicy, useResumeRunFromRecoveryAssumingFalsePositiveMutation, } from '@opentrons/react-api-client' -import { useChainRunCommands } from '/app/resources/runs' +import { + useChainRunCommands, + useUpdateRecoveryPolicyWithStrategy, +} from '/app/resources/runs' import { DEFINED_ERROR_TYPES, ERROR_KINDS, RECOVERY_MAP } from '../constants' import { getErrorKind } from '/app/organisms/ErrorRecoveryFlows/utils' @@ -23,12 +25,7 @@ import type { PrepareToAspirateRunTimeCommand, MoveLabwareParams, } from '@opentrons/shared-data' -import type { - CommandData, - IfMatchType, - RecoveryPolicyRulesParams, - RunAction, -} from '@opentrons/api-client' +import type { CommandData, IfMatchType, RunAction } from '@opentrons/api-client' import type { WellGroup } from '@opentrons/components' import type { FailedCommand, RecoveryRoute, RouteStep } from '../types' import type { UseFailedLabwareUtilsResult } from './useFailedLabwareUtils' @@ -38,6 +35,7 @@ import type { UseRecoveryAnalyticsResult } from '/app/redux-resources/analytics' import type { CurrentRecoveryOptionUtils } from './useRecoveryRouting' import type { ErrorRecoveryFlowsProps } from '..' import type { FailedCommandBySource } from './useRetainedFailedCommandBySource' +import type { UpdateErrorRecoveryPolicyWithStrategy } from '/app/resources/runs' interface UseRecoveryCommandsParams { runId: string @@ -100,11 +98,10 @@ export function useRecoveryCommands({ mutateAsync: resumeRunFromRecoveryAssumingFalsePositive, } = useResumeRunFromRecoveryAssumingFalsePositiveMutation() const { stopRun } = useStopRunMutation() - const { - mutateAsync: updateErrorRecoveryPolicy, - } = useUpdateErrorRecoveryPolicy(runId) + const updateErrorRecoveryPolicy = useUpdateRecoveryPolicyWithStrategy(runId) const { makeSuccessToast } = recoveryToastUtils + // TODO(jh, 11-21-24): Some commands return a 200 with an error body. We should catch these and propagate the error. const chainRunRecoveryCommands = useCallback( ( commands: CreateCommand[], @@ -230,10 +227,12 @@ export function useRecoveryCommands({ ifMatch ) - return updateErrorRecoveryPolicy(ignorePolicyRules) + return updateErrorRecoveryPolicy(ignorePolicyRules, 'append') .then(() => Promise.resolve()) - .catch(() => - Promise.reject(new Error('Failed to update recovery policy.')) + .catch((e: Error) => + Promise.reject( + new Error(`Failed to update recovery policy: ${e.message}`) + ) ) } else { void proceedToRouteAndStep(RECOVERY_MAP.ERROR_WHILE_RECOVERING.ROUTE) @@ -420,12 +419,8 @@ export const buildIgnorePolicyRules = ( commandType: FailedCommand['commandType'], errorType: string, ifMatch: IfMatchType -): RecoveryPolicyRulesParams => { - return [ - { - commandType, - errorType, - ifMatch, - }, - ] -} +): UpdateErrorRecoveryPolicyWithStrategy['newPolicy'] => ({ + commandType, + errorType, + ifMatch, +}) diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryToasts.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryToasts.ts index 6daf6998ae5..9fef84caca9 100644 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryToasts.ts +++ b/app/src/organisms/ErrorRecoveryFlows/hooks/useRecoveryToasts.ts @@ -110,8 +110,9 @@ export function useRecoveryFullCommandText( ): string | null { const { commandTextData, stepNumber } = props + // TODO TOME: I think you are looking one command to far, for some reason. const relevantCmdIdx = stepNumber ?? -1 - const relevantCmd = commandTextData?.commands[relevantCmdIdx] ?? null + const relevantCmd = commandTextData?.commands[relevantCmdIdx - 1] ?? null const { commandText, kind } = useCommandTextString({ ...props, diff --git a/app/src/organisms/ErrorRecoveryFlows/hooks/useRetainedFailedCommandBySource.ts b/app/src/organisms/ErrorRecoveryFlows/hooks/useRetainedFailedCommandBySource.ts index 90afa5851da..5a226fddcf1 100644 --- a/app/src/organisms/ErrorRecoveryFlows/hooks/useRetainedFailedCommandBySource.ts +++ b/app/src/organisms/ErrorRecoveryFlows/hooks/useRetainedFailedCommandBySource.ts @@ -1,4 +1,4 @@ -import { useState, useEffect } from 'react' +import { useState, useLayoutEffect } from 'react' import type { RunTimeCommand } from '@opentrons/shared-data' import type { ErrorRecoveryFlowsProps } from '..' @@ -27,7 +27,7 @@ export function useRetainedFailedCommandBySource( setRetainedFailedCommand, ] = useState(null) - useEffect(() => { + useLayoutEffect(() => { if (failedCommandByRunRecord !== null) { const failedCommandByAnalysis = protocolAnalysis?.commands.find( diff --git a/app/src/organisms/ErrorRecoveryFlows/index.tsx b/app/src/organisms/ErrorRecoveryFlows/index.tsx index 6461ae773fc..68184d71b46 100644 --- a/app/src/organisms/ErrorRecoveryFlows/index.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/index.tsx @@ -1,4 +1,4 @@ -import { useMemo, useState } from 'react' +import { useMemo, useLayoutEffect, useState } from 'react' import { useSelector } from 'react-redux' import { @@ -7,10 +7,12 @@ import { RUN_STATUS_AWAITING_RECOVERY_PAUSED, RUN_STATUS_BLOCKED_BY_OPEN_DOOR, RUN_STATUS_FAILED, + RUN_STATUS_FINISHING, RUN_STATUS_IDLE, RUN_STATUS_PAUSED, RUN_STATUS_RUNNING, RUN_STATUS_STOP_REQUESTED, + RUN_STATUS_STOPPED, RUN_STATUS_SUCCEEDED, } from '@opentrons/api-client' import { @@ -41,10 +43,13 @@ const VALID_ER_RUN_STATUSES: RunStatus[] = [ RUN_STATUS_STOP_REQUESTED, ] +// Effectively statuses that are not an "awaiting-recovery" status OR "stop requested." const INVALID_ER_RUN_STATUSES: RunStatus[] = [ RUN_STATUS_RUNNING, RUN_STATUS_PAUSED, RUN_STATUS_BLOCKED_BY_OPEN_DOOR, + RUN_STATUS_FINISHING, + RUN_STATUS_STOPPED, RUN_STATUS_FAILED, RUN_STATUS_SUCCEEDED, RUN_STATUS_IDLE, @@ -52,7 +57,6 @@ const INVALID_ER_RUN_STATUSES: RunStatus[] = [ export interface UseErrorRecoveryResult { isERActive: boolean - /* There is no FailedCommand if the run statis is not AWAITING_RECOVERY. */ failedCommand: FailedCommand | null } @@ -61,44 +65,38 @@ export function useErrorRecoveryFlows( runStatus: RunStatus | null ): UseErrorRecoveryResult { const [isERActive, setIsERActive] = useState(false) - // If client accesses a valid ER runs status besides AWAITING_RECOVERY but accesses it outside of Error Recovery flows, don't show ER. - const [hasSeenAwaitingRecovery, setHasSeenAwaitingRecovery] = useState(false) const failedCommand = useCurrentlyRecoveringFrom(runId, runStatus) - if ( - !hasSeenAwaitingRecovery && - ([ - RUN_STATUS_AWAITING_RECOVERY, - RUN_STATUS_AWAITING_RECOVERY_BLOCKED_BY_OPEN_DOOR, - RUN_STATUS_AWAITING_RECOVERY_PAUSED, - ] as Array).includes(runStatus) - ) { - setHasSeenAwaitingRecovery(true) + // The complexity of this logic exists to persist Error Recovery screens past the server's definition of Error Recovery. + // Ex, show a "cancelling run" modal in Error Recovery flows despite the robot no longer being in a recoverable state. + + const isValidERStatus = ( + status: RunStatus | null, + hasSeenAwaitingRecovery: boolean + ): boolean => { + return ( + status !== null && + (status === RUN_STATUS_AWAITING_RECOVERY || + (VALID_ER_RUN_STATUSES.includes(status) && hasSeenAwaitingRecovery)) + ) } - // Reset recovery mode after the client has exited recovery, otherwise "cancel run" will trigger ER after the first recovery. - else if ( - hasSeenAwaitingRecovery && - runStatus != null && - INVALID_ER_RUN_STATUSES.includes(runStatus) - ) { - setHasSeenAwaitingRecovery(false) - } - - const isValidRunStatus = - runStatus != null && - VALID_ER_RUN_STATUSES.includes(runStatus) && - hasSeenAwaitingRecovery - if (!isERActive && isValidRunStatus && failedCommand != null) { - setIsERActive(true) - } - // Because multiple ER flows may occur per run, disable ER when the status is not "awaiting-recovery" or a - // terminating run status in which we want to persist ER flows. Specific recovery commands cause run status to change. - // See a specific command's docstring for details. - // ER handles a null failedCommand outside the splash screen, so we shouldn't set it false here. - else if (isERActive && !isValidRunStatus) { - setIsERActive(false) - } + // If client accesses a valid ER runs status besides AWAITING_RECOVERY but accesses it outside of Error Recovery flows, + // don't show ER. + useLayoutEffect(() => { + if (runStatus != null) { + const isAwaitingRecovery = + VALID_ER_RUN_STATUSES.includes(runStatus) && + runStatus !== RUN_STATUS_STOP_REQUESTED && + failedCommand != null // Prevents one render cycle of an unknown failed command. + + if (isAwaitingRecovery) { + setIsERActive(isValidERStatus(runStatus, true)) + } else if (INVALID_ER_RUN_STATUSES.includes(runStatus)) { + setIsERActive(isValidERStatus(runStatus, false)) + } + } + }, [runStatus, failedCommand]) return { isERActive, diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/GripperIsHoldingLabware.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/GripperIsHoldingLabware.tsx index 94ef850834c..036f1aff3d0 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/GripperIsHoldingLabware.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/shared/GripperIsHoldingLabware.tsx @@ -14,11 +14,11 @@ import { import { RecoverySingleColumnContentWrapper } from './RecoveryContentWrapper' import { RecoveryFooterButtons } from './RecoveryFooterButtons' import { ODD_ONLY, DESKTOP_ONLY, RECOVERY_MAP } from '../constants' +import { RecoveryRadioGroup } from '/app/organisms/ErrorRecoveryFlows/shared/RecoveryRadioGroup' import type { JSX } from 'react' import type { TFunction } from 'i18next' import type { RecoveryContentProps } from '../types' -import { RecoveryRadioGroup } from '/app/organisms/ErrorRecoveryFlows/shared/RecoveryRadioGroup' type HoldingLabwareOption = 'yes' | 'no' export const HOLDING_LABWARE_OPTIONS: HoldingLabwareOption[] = [ diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/GripperReleaseLabware.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/GripperReleaseLabware.tsx index 3e0c24756d2..2c433775a83 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/GripperReleaseLabware.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/shared/GripperReleaseLabware.tsx @@ -95,6 +95,6 @@ const ANIMATION_CONTAINER_STYLE = css` const ANIMATION_STYLE = css` @media ${RESPONSIVENESS.touchscreenMediaQuerySpecs} { width: 27rem; - height: 18.75rem; + height: 20.25rem; } ` diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryInterventionModal.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryInterventionModal.tsx index ccfaa8376ba..82974023805 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryInterventionModal.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/shared/RecoveryInterventionModal.tsx @@ -63,6 +63,7 @@ const SMALL_MODAL_STYLE = css` padding: ${SPACING.spacing32}; height: 100%; overflow: ${OVERFLOW_HIDDEN}; + user-select: none; } ` const LARGE_MODAL_STYLE = css` @@ -73,5 +74,6 @@ const LARGE_MODAL_STYLE = css` @media ${RESPONSIVENESS.touchscreenMediaQuerySpecs} { height: 100%; overflow: ${OVERFLOW_HIDDEN}; + user-select: none; } ` diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/TwoColLwInfoAndDeck.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/TwoColLwInfoAndDeck.tsx index e378b25d769..00fa95072c1 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/TwoColLwInfoAndDeck.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/shared/TwoColLwInfoAndDeck.tsx @@ -102,9 +102,9 @@ export function TwoColLwInfoAndDeck( >['infoProps']['type'] => { switch (selectedRecoveryOption) { case MANUAL_MOVE_AND_SKIP.ROUTE: - case MANUAL_REPLACE_AND_RETRY.ROUTE: return 'location-arrow-location' default: + case MANUAL_REPLACE_AND_RETRY.ROUTE: return 'location' } } diff --git a/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/TwoColLwInfoAndDeck.test.tsx b/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/TwoColLwInfoAndDeck.test.tsx index 15094e5aacb..0629038f800 100644 --- a/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/TwoColLwInfoAndDeck.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/shared/__tests__/TwoColLwInfoAndDeck.test.tsx @@ -98,7 +98,7 @@ describe('TwoColLwInfoAndDeck', () => { expect(vi.mocked(LeftColumnLabwareInfo)).toHaveBeenCalledWith( expect.objectContaining({ title: 'Manually replace labware on deck', - type: 'location-arrow-location', + type: 'location', bannerText: 'Ensure labware is accurately placed in the slot to prevent further errors.', }), diff --git a/app/src/organisms/InterventionModal/PauseInterventionContent.tsx b/app/src/organisms/InterventionModal/PauseInterventionContent.tsx index 72f09e522b6..bb39eadb698 100644 --- a/app/src/organisms/InterventionModal/PauseInterventionContent.tsx +++ b/app/src/organisms/InterventionModal/PauseInterventionContent.tsx @@ -10,9 +10,8 @@ import { Flex, RESPONSIVENESS, SPACING, - TYPOGRAPHY, useInterval, - LegacyStyledText, + StyledText, } from '@opentrons/components' import { EMPTY_TIMESTAMP } from '/app/resources/runs' @@ -61,20 +60,6 @@ const PAUSE_HEADER_STYLE = css` } ` -const PAUSE_TEXT_STYLE = css` - ${TYPOGRAPHY.h1Default} - @media ${RESPONSIVENESS.touchscreenMediaQuerySpecs} { - ${TYPOGRAPHY.level4HeaderSemiBold} - } -` - -const PAUSE_TIME_STYLE = css` - ${TYPOGRAPHY.h1Default} - @media ${RESPONSIVENESS.touchscreenMediaQuerySpecs} { - ${TYPOGRAPHY.level1Header} - } -` - interface PauseHeaderProps { startedAt: string | null } @@ -95,10 +80,15 @@ function PauseHeader({ startedAt }: PauseHeaderProps): JSX.Element { return ( - + {i18n.format(t('paused_for'), 'capitalize')} - - {runTime} + + + {runTime} + ) } diff --git a/app/src/organisms/InterventionModal/index.tsx b/app/src/organisms/InterventionModal/index.tsx index 4e837b9458a..864bc99a231 100644 --- a/app/src/organisms/InterventionModal/index.tsx +++ b/app/src/organisms/InterventionModal/index.tsx @@ -165,7 +165,7 @@ export function InterventionModal({ } })() - const { iconName, headerTitle, headerTitleOnDevice } = (() => { + const { iconName, headerTitle, headerTitleOnDevice, iconSize } = (() => { switch (command.commandType) { case 'waitForResume': case 'pause': @@ -173,12 +173,14 @@ export function InterventionModal({ iconName: 'pause-circle' as IconName, headerTitle: t('pause_on', { robot_name: robotName }), headerTitleOnDevice: t('pause'), + iconSize: SPACING.spacing32, } case 'moveLabware': return { iconName: 'move-xy-circle' as IconName, headerTitle: t('move_labware_on', { robot_name: robotName }), headerTitleOnDevice: t('move_labware'), + iconSize: SPACING.spacing32, } default: console.warn( @@ -189,6 +191,7 @@ export function InterventionModal({ iconName: null, headerTitle: '', headerTitleOnDevice: '', + iconSize: undefined, } } })() @@ -228,6 +231,7 @@ export function InterventionModal({ iconHeading={{headerTitle}} iconName={iconName} type="intervention-required" + iconSize={iconSize} > {childContent} diff --git a/app/src/organisms/LabwarePositionCheck/CheckItem.tsx b/app/src/organisms/LabwarePositionCheck/CheckItem.tsx index c9050b5dd3f..5d5008554a6 100644 --- a/app/src/organisms/LabwarePositionCheck/CheckItem.tsx +++ b/app/src/organisms/LabwarePositionCheck/CheckItem.tsx @@ -156,6 +156,13 @@ export const CheckItem = (props: CheckItemProps): JSX.Element | null => { t as TFunction, i18n ) + const slotOnlyDisplayLocation = getDisplayLocation( + location, + labwareDefs, + t as TFunction, + i18n, + true + ) const labwareDisplayName = getLabwareDisplayName(labwareDef) let placeItemInstruction: JSX.Element = ( @@ -445,7 +452,7 @@ export const CheckItem = (props: CheckItemProps): JSX.Element | null => { { {...props} header={t('prepare_item_in_location', { item: isTiprack ? t('tip_rack') : t('labware'), - location: displayLocation, + location: slotOnlyDisplayLocation, })} body={ { const { moduleData } = props - const { t } = useTranslation('device_details') + const { t, i18n } = useTranslation(['device_details', 'shared']) const StatusLabelProps = { status: 'Idle', @@ -37,6 +37,10 @@ export const AbsorbanceReaderData = ( break } } + const lidDisplayStatus = + moduleData.lidStatus === 'on' + ? i18n.format(t('shared:closed'), 'capitalize') + : i18n.format(t('shared:open'), 'capitalize') return ( <> @@ -46,7 +50,7 @@ export const AbsorbanceReaderData = ( data-testid="abs_module_data" > {t('abs_reader_lid_status', { - status: moduleData.lidStatus === 'on' ? 'closed' : 'open', + status: lidDisplayStatus, })} diff --git a/app/src/organisms/ODD/ProtocolSetup/ProtocolSetupLiquids/index.tsx b/app/src/organisms/ODD/ProtocolSetup/ProtocolSetupLiquids/index.tsx index 551b7e716cd..153315d294b 100644 --- a/app/src/organisms/ODD/ProtocolSetup/ProtocolSetupLiquids/index.tsx +++ b/app/src/organisms/ODD/ProtocolSetup/ProtocolSetupLiquids/index.tsx @@ -153,26 +153,30 @@ export function LiquidsList(props: LiquidsListProps): JSX.Element { - {liquid.displayName} + {liquid.displayName.length > 33 + ? `${liquid.displayName.substring(0, 33)}...` + : liquid.displayName} - + {getTotalVolumePerLiquidId(liquid.id, labwareByLiquidId)}{' '} {MICRO_LITERS} + - {openItem ? ( { it('renders the first air gap screen, continue, and back buttons', () => { render(props) - screen.getByText('Air gap before aspirating') + screen.getByText('Air gap after aspirating') screen.getByTestId('ChildNavigation_Primary_Button') screen.getByText('Enabled') screen.getByText('Disabled') diff --git a/app/src/organisms/ODD/RunningProtocol/CurrentRunningProtocolCommand.tsx b/app/src/organisms/ODD/RunningProtocol/CurrentRunningProtocolCommand.tsx index 2866c3f95dd..92c1d1f5733 100644 --- a/app/src/organisms/ODD/RunningProtocol/CurrentRunningProtocolCommand.tsx +++ b/app/src/organisms/ODD/RunningProtocol/CurrentRunningProtocolCommand.tsx @@ -149,7 +149,6 @@ export function CurrentRunningProtocolCommand({ }: CurrentRunningProtocolCommandProps): JSX.Element | null { const { t } = useTranslation('run_details') const { data: mostRecentCommandData } = useNotifyAllCommandsQuery(runId, { - cursor: null, pageLength: 1, }) diff --git a/app/src/pages/ODD/RobotSettingsDashboard/RobotSettingsList.tsx b/app/src/pages/ODD/RobotSettingsDashboard/RobotSettingsList.tsx index 043b2b8b843..cbc0d68e353 100644 --- a/app/src/pages/ODD/RobotSettingsDashboard/RobotSettingsList.tsx +++ b/app/src/pages/ODD/RobotSettingsDashboard/RobotSettingsList.tsx @@ -200,7 +200,7 @@ export function RobotSettingsList(props: RobotSettingsListProps): JSX.Element { settingName={t('app_settings:error_recovery_mode')} dataTestId="RobotSettingButton_error_recovery_mode" settingInfo={t('app_settings:error_recovery_mode_description')} - iconName="recovery" + iconName="recovery-alt" rightElement={} onClick={toggleERSettings} /> diff --git a/app/src/pages/ODD/RobotSettingsDashboard/__tests__/RobotSettingsDashboard.test.tsx b/app/src/pages/ODD/RobotSettingsDashboard/__tests__/RobotSettingsDashboard.test.tsx index 00b70120809..9fa824ec0b9 100644 --- a/app/src/pages/ODD/RobotSettingsDashboard/__tests__/RobotSettingsDashboard.test.tsx +++ b/app/src/pages/ODD/RobotSettingsDashboard/__tests__/RobotSettingsDashboard.test.tsx @@ -113,7 +113,7 @@ describe('RobotSettingsDashboard', () => { screen.getByText('Robot System Version') screen.getByText('Network Settings') screen.getByText('Status LEDs') - screen.getByText('Error Recovery Mode') + screen.getByText('Recovery mode') screen.getByText( 'Control the strip of color lights on the front of the robot.' ) diff --git a/app/src/pages/ODD/RunSummary/index.tsx b/app/src/pages/ODD/RunSummary/index.tsx index 98a53f591d4..86aec6aaf87 100644 --- a/app/src/pages/ODD/RunSummary/index.tsx +++ b/app/src/pages/ODD/RunSummary/index.tsx @@ -147,15 +147,15 @@ export function RunSummary(): JSX.Element { const { closeCurrentRun } = useCloseCurrentRun() // Close the current run only if it's active and then execute the onSuccess callback. Prefer this wrapper over // closeCurrentRun directly, since the callback is swallowed if currentRun is null. - const closeCurrentRunIfValid = (onSuccess?: () => void): void => { + const closeCurrentRunIfValid = (onSettled?: () => void): void => { if (isRunCurrent) { closeCurrentRun({ - onSuccess: () => { - onSuccess?.() + onSettled: () => { + onSettled?.() }, }) } else { - onSuccess?.() + onSettled?.() } } const [showRunFailedModal, setShowRunFailedModal] = useState(false) @@ -240,15 +240,14 @@ export function RunSummary(): JSX.Element { const runSummaryNoFixit = useCurrentRunCommands({ includeFixitCommands: false, pageLength: 1, - cursor: null, }) useEffect(() => { if ( isRunCurrent && runSummaryNoFixit != null && + runSummaryNoFixit.length > 0 && !lastRunCommandPromptedErrorRecovery(runSummaryNoFixit) ) { - console.log('HITTING THIS') void determineTipStatus() } }, [runSummaryNoFixit, isRunCurrent]) diff --git a/app/src/pages/ODD/RunningProtocol/__tests__/RunningProtocol.test.tsx b/app/src/pages/ODD/RunningProtocol/__tests__/RunningProtocol.test.tsx index f98666d3cbd..33d6f79e9a7 100644 --- a/app/src/pages/ODD/RunningProtocol/__tests__/RunningProtocol.test.tsx +++ b/app/src/pages/ODD/RunningProtocol/__tests__/RunningProtocol.test.tsx @@ -146,7 +146,6 @@ describe('RunningProtocol', () => { .thenReturn(mockRobotSideAnalysis) when(vi.mocked(useNotifyAllCommandsQuery)) .calledWith(RUN_ID, { - cursor: null, pageLength: 1, }) .thenReturn(mockUseAllCommandsResponseNonDeterministic) diff --git a/app/src/resources/instruments/useTipAttachmentStatus.ts b/app/src/resources/instruments/useTipAttachmentStatus.ts index ee0d6449ea6..b08d5ea5270 100644 --- a/app/src/resources/instruments/useTipAttachmentStatus.ts +++ b/app/src/resources/instruments/useTipAttachmentStatus.ts @@ -81,7 +81,6 @@ export function useTipAttachmentStatus( getCommands(host as HostConfig, runId, { includeFixitCommands: false, pageLength: 1, - cursor: null, }), ]) .then(([attachedInstruments, currentState, commandsData]) => { diff --git a/app/src/resources/protocols/hooks/useRunningStepCounts.ts b/app/src/resources/protocols/hooks/useRunningStepCounts.ts index 915676ba5ec..f3a7cb5028f 100644 --- a/app/src/resources/protocols/hooks/useRunningStepCounts.ts +++ b/app/src/resources/protocols/hooks/useRunningStepCounts.ts @@ -3,6 +3,7 @@ import { useMostRecentCompletedAnalysis } from '/app/resources/runs' import { useLastRunProtocolCommand } from './useLastRunProtocolCommand' import type { CommandsData } from '@opentrons/api-client' +import { getRunningStepCountsFrom } from '/app/resources/protocols' export interface StepCounts { /* Excludes "fixit" commands. Returns null if the step is not found. */ @@ -35,21 +36,5 @@ export function useRunningStepCounts( commandsData ?? null ) - const lastRunAnalysisCommandIndex = analysisCommands.findIndex( - c => c.key === lastRunCommandNoFixit?.key - ) - - const currentStepNumberByAnalysis = - lastRunAnalysisCommandIndex === -1 ? null : lastRunAnalysisCommandIndex + 1 - - const hasRunDiverged = - lastRunCommandNoFixit?.key == null || currentStepNumberByAnalysis == null - - const totalStepCount = !hasRunDiverged ? analysisCommands.length : null - - return { - currentStepNumber: currentStepNumberByAnalysis, - totalStepCount, - hasRunDiverged, - } + return getRunningStepCountsFrom(analysisCommands, lastRunCommandNoFixit) } diff --git a/app/src/resources/protocols/utils.ts b/app/src/resources/protocols/utils.ts index 82d62e8cc0b..cfee1b61eb6 100644 --- a/app/src/resources/protocols/utils.ts +++ b/app/src/resources/protocols/utils.ts @@ -1,4 +1,6 @@ import type { RunTimeCommand } from '@opentrons/shared-data' +import type { RunCommandSummary } from '@opentrons/api-client' +import type { StepCounts } from '/app/resources/protocols/hooks' export function isGripperInCommands(commands: RunTimeCommand[]): boolean { return ( @@ -8,3 +10,27 @@ export function isGripperInCommands(commands: RunTimeCommand[]): boolean { ) ?? false ) } + +// See useRunningStepCounts. +export function getRunningStepCountsFrom( + analysisCommands: RunTimeCommand[], + lastRunProtocolCommand: RunCommandSummary | null +): StepCounts { + const lastRunAnalysisCommandIndex = analysisCommands.findIndex( + c => c.key === lastRunProtocolCommand?.key + ) + + const currentStepNumberByAnalysis = + lastRunAnalysisCommandIndex === -1 ? null : lastRunAnalysisCommandIndex + 1 + + const hasRunDiverged = + lastRunProtocolCommand?.key == null || currentStepNumberByAnalysis == null + + const totalStepCount = !hasRunDiverged ? analysisCommands.length : null + + return { + currentStepNumber: currentStepNumberByAnalysis, + totalStepCount, + hasRunDiverged, + } +} diff --git a/app/src/resources/runs/__tests__/useRunTimestamps.test.ts b/app/src/resources/runs/__tests__/useRunTimestamps.test.ts index 1417be1e96d..cbe714472e9 100644 --- a/app/src/resources/runs/__tests__/useRunTimestamps.test.ts +++ b/app/src/resources/runs/__tests__/useRunTimestamps.test.ts @@ -24,7 +24,7 @@ vi.mock('../useNotifyRunQuery') describe('useRunTimestamps hook', () => { beforeEach(() => { when(useRunCommands) - .calledWith(RUN_ID_2, { cursor: null, pageLength: 1 }, expect.any(Object)) + .calledWith(RUN_ID_2, { pageLength: 1 }, expect.any(Object)) .thenReturn([mockCommand.data as any]) }) diff --git a/app/src/resources/runs/index.ts b/app/src/resources/runs/index.ts index 5c97434dd31..d1d84d3bc1b 100644 --- a/app/src/resources/runs/index.ts +++ b/app/src/resources/runs/index.ts @@ -29,3 +29,4 @@ export * from './useModuleCalibrationStatus' export * from './useProtocolAnalysisErrors' export * from './useLastRunCommand' export * from './useRunStatuses' +export * from './useUpdateRecoveryPolicyWithStrategy' diff --git a/app/src/resources/runs/useLastRunCommand.ts b/app/src/resources/runs/useLastRunCommand.ts index b061b46c7c8..a8ee6e249d3 100644 --- a/app/src/resources/runs/useLastRunCommand.ts +++ b/app/src/resources/runs/useLastRunCommand.ts @@ -18,7 +18,7 @@ export function useLastRunCommand( const runStatus = useRunStatus(runId) const { data: commandsData } = useNotifyAllCommandsQuery( runId, - { cursor: null, pageLength: 1 }, + { pageLength: 1 }, { ...options, refetchInterval: diff --git a/app/src/resources/runs/useNotifyAllCommandsQuery.ts b/app/src/resources/runs/useNotifyAllCommandsQuery.ts index 12bafb21ef3..e0082d26dd2 100644 --- a/app/src/resources/runs/useNotifyAllCommandsQuery.ts +++ b/app/src/resources/runs/useNotifyAllCommandsQuery.ts @@ -3,23 +3,12 @@ import { useAllCommandsQuery } from '@opentrons/react-api-client' import { useNotifyDataReady } from '../useNotifyDataReady' import type { UseQueryResult } from 'react-query' -import type { - CommandsData, - GetRunCommandsParams, - GetCommandsParams, -} from '@opentrons/api-client' +import type { CommandsData, GetRunCommandsParams } from '@opentrons/api-client' import type { QueryOptionsWithPolling } from '../useNotifyDataReady' -const DEFAULT_PAGE_LENGTH = 30 - -export const DEFAULT_PARAMS: GetCommandsParams = { - cursor: null, - pageLength: DEFAULT_PAGE_LENGTH, -} - export function useNotifyAllCommandsQuery( runId: string | null, - params?: GetRunCommandsParams | null, + params?: GetRunCommandsParams, options: QueryOptionsWithPolling = {} ): UseQueryResult { // Assume the useAllCommandsQuery() response can only change when the command links change. @@ -32,19 +21,8 @@ export function useNotifyAllCommandsQuery( topic: 'robot-server/runs/commands_links', options, }) - const nullCheckedParams = params ?? DEFAULT_PARAMS - - const nullCheckedFixitCommands = params?.includeFixitCommands ?? null - const finalizedNullCheckParams = { - ...nullCheckedParams, - includeFixitCommands: nullCheckedFixitCommands, - } - const httpQueryResult = useAllCommandsQuery( - runId, - finalizedNullCheckParams, - queryOptionsNotify - ) + const httpQueryResult = useAllCommandsQuery(runId, params, queryOptionsNotify) if (shouldRefetch) { void httpQueryResult.refetch() diff --git a/app/src/resources/runs/useRunTimestamps.ts b/app/src/resources/runs/useRunTimestamps.ts index f3ae3f1a803..b19cbe42193 100644 --- a/app/src/resources/runs/useRunTimestamps.ts +++ b/app/src/resources/runs/useRunTimestamps.ts @@ -30,7 +30,7 @@ export function useRunTimestamps(runId: string | null): RunTimestamps { const runCommands = useRunCommands( runId, - { cursor: null, pageLength: 1 }, + { pageLength: 1 }, { enabled: runStatus === RUN_STATUS_SUCCEEDED || diff --git a/app/src/resources/runs/useUpdateRecoveryPolicyWithStrategy.ts b/app/src/resources/runs/useUpdateRecoveryPolicyWithStrategy.ts new file mode 100644 index 00000000000..f26eb507afc --- /dev/null +++ b/app/src/resources/runs/useUpdateRecoveryPolicyWithStrategy.ts @@ -0,0 +1,60 @@ +import { + useHost, + useUpdateErrorRecoveryPolicy, +} from '@opentrons/react-api-client' +import { getErrorRecoveryPolicy } from '@opentrons/api-client' + +import type { + HostConfig, + RecoveryPolicyRulesParams, + UpdateErrorRecoveryPolicyResponse, +} from '@opentrons/api-client' + +/** + * append - Add a new policy rule to the end of the existing recovery policy. + */ +export type UpdatePolicyStrategy = 'append' + +export interface UpdateErrorRecoveryPolicyWithStrategy { + runId: string + newPolicy: RecoveryPolicyRulesParams[number] + strategy: UpdatePolicyStrategy +} + +export function useUpdateRecoveryPolicyWithStrategy( + runId: string +): ( + newPolicy: UpdateErrorRecoveryPolicyWithStrategy['newPolicy'], + strategy: UpdateErrorRecoveryPolicyWithStrategy['strategy'] +) => Promise { + const host = useHost() + + const { + mutateAsync: updateErrorRecoveryPolicy, + } = useUpdateErrorRecoveryPolicy(runId) + + return ( + newPolicy: UpdateErrorRecoveryPolicyWithStrategy['newPolicy'], + strategy: UpdateErrorRecoveryPolicyWithStrategy['strategy'] + ) => + getErrorRecoveryPolicy(host as HostConfig, runId).then(res => { + const existingPolicyRules = res.data.data.policyRules.map(rule => ({ + commandType: rule.matchCriteria.command.commandType, + errorType: rule.matchCriteria.command.error.errorType, + ifMatch: rule.ifMatch, + })) + + const buildUpdatedPolicy = (): RecoveryPolicyRulesParams => { + switch (strategy) { + case 'append': + return [...existingPolicyRules, newPolicy] + default: { + console.error('Unhandled policy strategy, defaulting to append.') + return [...existingPolicyRules, newPolicy] + } + } + } + + return updateErrorRecoveryPolicy(buildUpdatedPolicy()) + }) +} diff --git a/components/src/atoms/InputField/index.tsx b/components/src/atoms/InputField/index.tsx index 40642a897e0..001374f71ce 100644 --- a/components/src/atoms/InputField/index.tsx +++ b/components/src/atoms/InputField/index.tsx @@ -254,13 +254,13 @@ export const InputField = forwardRef( color: ${props.disabled ? COLORS.grey40 : COLORS.grey50}; font: ${TYPOGRAPHY.bodyTextRegular}; text-align: ${TYPOGRAPHY.textAlignRight}; + white-space: ${NO_WRAP}; @media ${RESPONSIVENESS.touchscreenMediaQuerySpecs} { color: ${props.disabled ? COLORS.grey40 : COLORS.grey50}; font-size: ${TYPOGRAPHY.fontSize22}; font-weight: ${TYPOGRAPHY.fontWeightRegular}; line-height: ${TYPOGRAPHY.lineHeight28}; justify-content: ${textAlign}; - white-space: ${NO_WRAP}; } ` diff --git a/components/src/icons/icon-data.ts b/components/src/icons/icon-data.ts index b5e928005dc..c37534a7df6 100644 --- a/components/src/icons/icon-data.ts +++ b/components/src/icons/icon-data.ts @@ -655,6 +655,11 @@ export const ICON_DATA_BY_NAME: Record< 'M24.4 49.6H34.2V38.6H44.2V29.8H34.2V18.8H24.4V29.8H13.4V38.6H24.4V49.6ZM28.8 72C20.46 69.9 13.575 65.115 8.145 57.645C2.715 50.175 0 41.88 0 32.76V10.8L28.8 0L57.6 10.8V32.76C57.6 41.88 54.885 50.175 49.455 57.645C44.025 65.115 37.14 69.9 28.8 72Z', viewBox: '0 0 58 72', }, + 'recovery-alt': { + path: + 'M14.5 27.2563H17.5V11.4062H14.5V27.2563ZM16 40.4062C11.3333 39.2396 7.5 36.5312 4.5 32.2812C1.5 28.0312 0 23.3729 0 18.3062V6.40625L16 0.40625L32 6.40625V18.3062C32 23.3729 30.5 28.0312 27.5 32.2812C24.5 36.5312 20.6667 39.2396 16 40.4062ZM16 37.3063C19.8333 36.0396 22.9583 33.6479 25.375 30.1313C27.7917 26.6146 29 22.6729 29 18.3062V8.50625L16 3.60625L3 8.50625V18.3062C3 22.6729 4.20833 26.6146 6.625 30.1313C9.04167 33.6479 12.1667 36.0396 16 37.3063Z M23.85 17.9062V20.9062H8V17.9062H23.85Z M17.5 17.9062H14.5V20.9062H17.5V17.9062Z', + viewBox: '0 0 32 41', + }, 'radiobox-blank': { path: 'M12,20A8,8 0 0,1 4,12A8,8 0 0,1 12,4A8,8 0 0,1 20,12A8,8 0 0,1 12,20M12,2A10,10 0 0,0 2,12A10,10 0 0,0 12,22A10,10 0 0,0 22,12A10,10 0 0,0 12,2Z', diff --git a/react-api-client/src/runs/index.ts b/react-api-client/src/runs/index.ts index 5e479ed5093..77b487a84fc 100644 --- a/react-api-client/src/runs/index.ts +++ b/react-api-client/src/runs/index.ts @@ -19,6 +19,7 @@ export { useRunCommandErrors } from './useRunCommandErrors' export * from './useCreateLabwareOffsetMutation' export * from './useCreateLabwareDefinitionMutation' export * from './useUpdateErrorRecoveryPolicy' +export * from './useErrorRecoveryPolicy' export type { UsePlayRunMutationResult } from './usePlayRunMutation' export type { UsePauseRunMutationResult } from './usePauseRunMutation' diff --git a/react-api-client/src/runs/useAllCommandsAsPreSerializedList.ts b/react-api-client/src/runs/useAllCommandsAsPreSerializedList.ts index 4f7e2fdc0e0..c441fcc3553 100644 --- a/react-api-client/src/runs/useAllCommandsAsPreSerializedList.ts +++ b/react-api-client/src/runs/useAllCommandsAsPreSerializedList.ts @@ -14,10 +14,6 @@ import type { } from '@opentrons/api-client' const DEFAULT_PAGE_LENGTH = 30 -export const DEFAULT_PARAMS: GetRunCommandsParams = { - cursor: null, - pageLength: DEFAULT_PAGE_LENGTH, -} export function useAllCommandsAsPreSerializedList( runId: string | null, @@ -25,17 +21,15 @@ export function useAllCommandsAsPreSerializedList( options: UseQueryOptions = {} ): UseQueryResult { const host = useHost() - const nullCheckedParams = params ?? DEFAULT_PARAMS const allOptions: UseQueryOptions = { ...options, enabled: host !== null && runId != null && options.enabled !== false, } - const { cursor, pageLength } = nullCheckedParams - const nullCheckedFixitCommands = params?.includeFixitCommands ?? null - const finalizedNullCheckParams = { - ...nullCheckedParams, - includeFixitCommands: nullCheckedFixitCommands, + const { cursor, pageLength, includeFixitCommands } = params ?? {} + const finalizedParams = { + ...params, + pageLength: params?.pageLength ?? DEFAULT_PAGE_LENGTH, } // map undefined values to null to agree with react query caching @@ -50,13 +44,13 @@ export function useAllCommandsAsPreSerializedList( 'getCommandsAsPreSerializedList', cursor, pageLength, - nullCheckedFixitCommands, + includeFixitCommands, ], () => { return getCommandsAsPreSerializedList( host as HostConfig, runId as string, - finalizedNullCheckParams + finalizedParams ).then(response => { const responseData = response.data return { diff --git a/react-api-client/src/runs/useAllCommandsQuery.ts b/react-api-client/src/runs/useAllCommandsQuery.ts index 427e96b554f..a1fd6047fe0 100644 --- a/react-api-client/src/runs/useAllCommandsQuery.ts +++ b/react-api-client/src/runs/useAllCommandsQuery.ts @@ -9,45 +9,30 @@ import type { } from '@opentrons/api-client' const DEFAULT_PAGE_LENGTH = 30 -export const DEFAULT_PARAMS: GetRunCommandsParamsRequest = { - cursor: null, - pageLength: DEFAULT_PAGE_LENGTH, - includeFixitCommands: null, -} export function useAllCommandsQuery( runId: string | null, - params?: GetRunCommandsParamsRequest | null, + params?: GetRunCommandsParamsRequest, options: UseQueryOptions = {} ): UseQueryResult { const host = useHost() - const nullCheckedParams = params ?? DEFAULT_PARAMS - const allOptions: UseQueryOptions = { ...options, enabled: host !== null && runId != null && options.enabled !== false, } - const { cursor, pageLength } = nullCheckedParams - const nullCheckedFixitCommands = params?.includeFixitCommands ?? null - const finalizedNullCheckParams = { - ...nullCheckedParams, - includeFixitCommands: nullCheckedFixitCommands, + + const { cursor, pageLength, includeFixitCommands } = params ?? {} + const finalizedParams = { + ...params, + pageLength: params?.pageLength ?? DEFAULT_PAGE_LENGTH, } const query = useQuery( - [ - host, - 'runs', - runId, - 'commands', - cursor, - pageLength, - finalizedNullCheckParams, - ], + [host, 'runs', runId, 'commands', cursor, pageLength, includeFixitCommands], () => { return getCommands( host as HostConfig, runId as string, - finalizedNullCheckParams + finalizedParams ).then(response => response.data) }, allOptions diff --git a/react-api-client/src/runs/useErrorRecoveryPolicy.ts b/react-api-client/src/runs/useErrorRecoveryPolicy.ts new file mode 100644 index 00000000000..dd625689b83 --- /dev/null +++ b/react-api-client/src/runs/useErrorRecoveryPolicy.ts @@ -0,0 +1,31 @@ +import { useQuery } from 'react-query' + +import { getErrorRecoveryPolicy } from '@opentrons/api-client' + +import { useHost } from '../api' + +import type { UseQueryOptions, UseQueryResult } from 'react-query' +import type { + ErrorRecoveryPolicyResponse, + HostConfig, +} from '@opentrons/api-client' + +export function useErrorRecoveryPolicy( + runId: string, + options: UseQueryOptions = {} +): UseQueryResult { + const host = useHost() + + const query = useQuery( + [host, 'runs', runId, 'errorRecoveryPolicy'], + () => + getErrorRecoveryPolicy(host as HostConfig, runId) + .then(response => response.data) + .catch(e => { + throw e + }), + options + ) + + return query +} diff --git a/react-api-client/src/runs/useRunCommandErrors.ts b/react-api-client/src/runs/useRunCommandErrors.ts index 63dd5875636..ef536d438e0 100644 --- a/react-api-client/src/runs/useRunCommandErrors.ts +++ b/react-api-client/src/runs/useRunCommandErrors.ts @@ -9,10 +9,6 @@ import type { } from '@opentrons/api-client' const DEFAULT_PAGE_LENGTH = 30 -export const DEFAULT_PARAMS: GetCommandsParams = { - cursor: null, - pageLength: DEFAULT_PAGE_LENGTH, -} export function useRunCommandErrors( runId: string | null, @@ -20,20 +16,23 @@ export function useRunCommandErrors( options: UseQueryOptions = {} ): UseQueryResult { const host = useHost() - const nullCheckedParams = params ?? DEFAULT_PARAMS - const allOptions: UseQueryOptions = { ...options, enabled: host !== null && runId != null && options.enabled !== false, } - const { cursor, pageLength } = nullCheckedParams + + const { cursor, pageLength } = params ?? {} + const finalizedParams = { + ...params, + pageLength: params?.pageLength ?? DEFAULT_PAGE_LENGTH, + } const query = useQuery( [host, 'runs', runId, 'commandErrors', cursor, pageLength], () => { return getRunCommandErrors( host as HostConfig, runId as string, - nullCheckedParams + finalizedParams ).then(response => response.data) }, allOptions diff --git a/react-api-client/src/runs/useUpdateErrorRecoveryPolicy.ts b/react-api-client/src/runs/useUpdateErrorRecoveryPolicy.ts index 1fa379b1bc5..7e09abc6160 100644 --- a/react-api-client/src/runs/useUpdateErrorRecoveryPolicy.ts +++ b/react-api-client/src/runs/useUpdateErrorRecoveryPolicy.ts @@ -16,7 +16,7 @@ import type { HostConfig, } from '@opentrons/api-client' -export type UseUpdateErrorRecoveryPolicyResponse = UseMutationResult< +export type UseErrorRecoveryPolicyResponse = UseMutationResult< UpdateErrorRecoveryPolicyResponse, AxiosError, RecoveryPolicyRulesParams @@ -37,7 +37,7 @@ export type UseUpdateErrorRecoveryPolicyOptions = UseMutationOptions< export function useUpdateErrorRecoveryPolicy( runId: string, options: UseUpdateErrorRecoveryPolicyOptions = {} -): UseUpdateErrorRecoveryPolicyResponse { +): UseErrorRecoveryPolicyResponse { const host = useHost() const mutation = useMutation< diff --git a/robot-server/robot_server/runs/dependencies.py b/robot-server/robot_server/runs/dependencies.py index 60c96d1e5e6..5f49f04582d 100644 --- a/robot-server/robot_server/runs/dependencies.py +++ b/robot-server/robot_server/runs/dependencies.py @@ -48,6 +48,7 @@ _run_orchestrator_store_accessor = AppStateAccessor[RunOrchestratorStore]( "run_orchestrator_store" ) +_run_data_manager_accessor = AppStateAccessor[RunDataManager]("run_data_manager") _light_control_accessor = AppStateAccessor[LightController]("light_controller") @@ -154,6 +155,7 @@ async def get_is_okay_to_create_maintenance_run( async def get_run_data_manager( + app_state: Annotated[AppState, Depends(get_app_state)], task_runner: Annotated[TaskRunner, Depends(get_task_runner)], run_orchestrator_store: Annotated[ RunOrchestratorStore, Depends(get_run_orchestrator_store) @@ -164,14 +166,20 @@ async def get_run_data_manager( ErrorRecoverySettingStore, Depends(get_error_recovery_setting_store) ], ) -> RunDataManager: - """Get a run data manager to keep track of current/historical run data.""" - return RunDataManager( - run_orchestrator_store=run_orchestrator_store, - run_store=run_store, - error_recovery_setting_store=error_recovery_setting_store, - task_runner=task_runner, - runs_publisher=runs_publisher, - ) + """Get a singleton run data manager to keep track of current/historical run data.""" + run_data_manager = _run_data_manager_accessor.get_from(app_state) + + if run_data_manager is None: + run_data_manager = RunDataManager( + run_orchestrator_store=run_orchestrator_store, + run_store=run_store, + error_recovery_setting_store=error_recovery_setting_store, + task_runner=task_runner, + runs_publisher=runs_publisher, + ) + _run_data_manager_accessor.set_on(app_state, run_data_manager) + + return run_data_manager async def get_run_auto_deleter( diff --git a/robot-server/robot_server/runs/router/__init__.py b/robot-server/robot_server/runs/router/__init__.py index 738d0e05952..d7a1640edb8 100644 --- a/robot-server/robot_server/runs/router/__init__.py +++ b/robot-server/robot_server/runs/router/__init__.py @@ -5,6 +5,7 @@ from .commands_router import commands_router from .actions_router import actions_router from .labware_router import labware_router +from .error_recovery_policy_router import error_recovery_policy_router runs_router = APIRouter() @@ -12,5 +13,6 @@ runs_router.include_router(commands_router) runs_router.include_router(actions_router) runs_router.include_router(labware_router) +runs_router.include_router(error_recovery_policy_router) __all__ = ["runs_router"] diff --git a/robot-server/robot_server/runs/router/base_router.py b/robot-server/robot_server/runs/router/base_router.py index a57ed636647..4fb4be2b401 100644 --- a/robot-server/robot_server/runs/router/base_router.py +++ b/robot-server/robot_server/runs/router/base_router.py @@ -75,7 +75,6 @@ get_run_auto_deleter, get_quick_transfer_run_auto_deleter, ) -from ..error_recovery_models import ErrorRecoveryPolicy from robot_server.deck_configuration.fastapi_dependencies import ( get_deck_configuration_store, @@ -439,47 +438,6 @@ async def update_run( ) -@PydanticResponse.wrap_route( - base_router.put, - path="/runs/{runId}/errorRecoveryPolicy", - summary="Set a run's error recovery policy", - description=dedent( - """ - Update how to handle different kinds of command failures. - - For this to have any effect, error recovery must also be enabled globally. - See `PATCH /errorRecovery/settings`. - """ - ), - responses={ - status.HTTP_200_OK: {"model": SimpleEmptyBody}, - status.HTTP_409_CONFLICT: {"model": ErrorBody[RunStopped]}, - }, -) -async def put_error_recovery_policy( - runId: str, - request_body: RequestModel[ErrorRecoveryPolicy], - run_data_manager: Annotated[RunDataManager, Depends(get_run_data_manager)], -) -> PydanticResponse[SimpleEmptyBody]: - """Create run polices. - - Arguments: - runId: Run ID pulled from URL. - request_body: Request body with run policies data. - run_data_manager: Current and historical run data management. - """ - rules = request_body.data.policyRules - try: - run_data_manager.set_error_recovery_rules(run_id=runId, rules=rules) - except RunNotCurrentError as e: - raise RunStopped(detail=str(e)).as_error(status.HTTP_409_CONFLICT) from e - - return await PydanticResponse.create( - content=SimpleEmptyBody.construct(), - status_code=status.HTTP_200_OK, - ) - - @PydanticResponse.wrap_route( base_router.get, path="/runs/{runId}/commandErrors", diff --git a/robot-server/robot_server/runs/router/error_recovery_policy_router.py b/robot-server/robot_server/runs/router/error_recovery_policy_router.py new file mode 100644 index 00000000000..4653d564244 --- /dev/null +++ b/robot-server/robot_server/runs/router/error_recovery_policy_router.py @@ -0,0 +1,97 @@ +"""Router for /runs/{runId}/errorRecoveryPolicy endpoints.""" + + +from textwrap import dedent +from typing import Annotated + +from fastapi import status, APIRouter, Depends + +from robot_server.errors.error_responses import ErrorBody +from robot_server.service.json_api.request import RequestModel +from robot_server.service.json_api.response import ( + PydanticResponse, + SimpleBody, + SimpleEmptyBody, +) + +from .base_router import RunStopped +from ..dependencies import get_run_data_manager +from ..run_data_manager import RunDataManager, RunNotCurrentError +from ..error_recovery_models import ErrorRecoveryPolicy + + +error_recovery_policy_router = APIRouter() + + +@PydanticResponse.wrap_route( + error_recovery_policy_router.put, + path="/runs/{runId}/errorRecoveryPolicy", + summary="Set a run's error recovery policy", + description=dedent( + """ + Update how to handle different kinds of command failures. + + For this to have any effect, error recovery must also be enabled globally. + See `PATCH /errorRecovery/settings`. + """ + ), + responses={ + status.HTTP_200_OK: {"model": SimpleEmptyBody}, + status.HTTP_409_CONFLICT: {"model": ErrorBody[RunStopped]}, + }, +) +async def put_error_recovery_policy( + runId: str, + request_body: RequestModel[ErrorRecoveryPolicy], + run_data_manager: Annotated[RunDataManager, Depends(get_run_data_manager)], +) -> PydanticResponse[SimpleEmptyBody]: + """Set a run's error recovery policy. + + Arguments: + runId: Run ID pulled from URL. + request_body: Request body with the new run policy. + run_data_manager: Current and historical run data management. + """ + rules = request_body.data.policyRules + try: + run_data_manager.set_error_recovery_rules(run_id=runId, rules=rules) + except RunNotCurrentError as e: + raise RunStopped(detail=str(e)).as_error(status.HTTP_409_CONFLICT) from e + + return await PydanticResponse.create( + content=SimpleEmptyBody.construct(), + status_code=status.HTTP_200_OK, + ) + + +@PydanticResponse.wrap_route( + error_recovery_policy_router.get, + path="/runs/{runId}/errorRecoveryPolicy", + summary="Get a run's current error recovery policy", + description="See `PUT /runs/{runId}/errorRecoveryPolicy`.", + responses={ + status.HTTP_200_OK: {"model": SimpleBody[ErrorRecoveryPolicy]}, + status.HTTP_409_CONFLICT: {"model": ErrorBody[RunStopped]}, + }, +) +async def get_error_recovery_policy( + runId: str, + run_data_manager: Annotated[RunDataManager, Depends(get_run_data_manager)], +) -> PydanticResponse[SimpleBody[ErrorRecoveryPolicy]]: + """Get a run's current error recovery policy. + + Arguments: + runId: Run ID pulled from URL. + run_data_manager: Current and historical run data management. + """ + try: + rules = run_data_manager.get_error_recovery_rules(run_id=runId) + except RunNotCurrentError as e: + raise RunStopped(detail=str(e)).as_error(status.HTTP_409_CONFLICT) from e + + return await PydanticResponse.create( + content=SimpleBody.construct( + data=ErrorRecoveryPolicy.construct(policyRules=rules) + ), + status_code=status.HTTP_200_OK, + ) diff --git a/robot-server/robot_server/runs/run_data_manager.py b/robot-server/robot_server/runs/run_data_manager.py index f5a06fa8172..9999e040523 100644 --- a/robot-server/robot_server/runs/run_data_manager.py +++ b/robot-server/robot_server/runs/run_data_manager.py @@ -143,7 +143,7 @@ class PreSerializedCommandsNotAvailableError(LookupError): class RunDataManager: """Collaborator to manage current and historical run data. - Provides a facade to both an EngineStore (current run) and a RunStore + Provides a facade to both a RunOrchestratorStore (current run) and a RunStore (historical runs). Returns `Run` response models to the router. Args: @@ -161,7 +161,14 @@ def __init__( ) -> None: self._run_orchestrator_store = run_orchestrator_store self._run_store = run_store + self._error_recovery_setting_store = error_recovery_setting_store + # todo(mm, 2024-11-22): Storing the list of error recovery rules is outside the + # responsibilities of this class. It's also clunky for us to store it like this + # because we need to remember to clear it whenever the current run changes. + # This error recovery mapping stuff probably belongs in RunOrchestratorStore. + self._current_run_error_recovery_rules: List[ErrorRecoveryRule] = [] + self._task_runner = task_runner self._runs_publisher = runs_publisher @@ -213,9 +220,10 @@ async def create( ) error_recovery_is_enabled = self._error_recovery_setting_store.get_is_enabled() + self._current_run_error_recovery_rules = _INITIAL_ERROR_RECOVERY_RULES initial_error_recovery_policy = ( error_recovery_mapping.create_error_recovery_policy_from_rules( - _INITIAL_ERROR_RECOVERY_RULES, error_recovery_is_enabled + self._current_run_error_recovery_rules, error_recovery_is_enabled ) ) @@ -539,7 +547,7 @@ def get_all_commands_as_preserialized_list( def set_error_recovery_rules( self, run_id: str, rules: List[ErrorRecoveryRule] ) -> None: - """Set the run's error recovery policy. + """Set the run's error recovery policy, in robot-server terms. The input rules get combined with the global error recovery enabled/disabled setting, which this method retrieves automatically. @@ -549,11 +557,20 @@ def set_error_recovery_rules( f"Cannot update {run_id} because it is not the current run." ) is_enabled = self._error_recovery_setting_store.get_is_enabled() + self._current_run_error_recovery_rules = rules mapped_policy = error_recovery_mapping.create_error_recovery_policy_from_rules( - rules, is_enabled + self._current_run_error_recovery_rules, is_enabled ) self._run_orchestrator_store.set_error_recovery_policy(policy=mapped_policy) + def get_error_recovery_rules(self, run_id: str) -> List[ErrorRecoveryRule]: + """Get the run's error recovery policy.""" + if run_id != self._run_orchestrator_store.current_run_id: + raise RunNotCurrentError( + f"Cannot get the error recovery policy of {run_id} because it is not the current run." + ) + return self._current_run_error_recovery_rules + def _get_state_summary(self, run_id: str) -> Union[StateSummary, BadStateSummary]: if run_id == self._run_orchestrator_store.current_run_id: return self._run_orchestrator_store.get_state_summary() diff --git a/robot-server/tests/runs/router/test_base_router.py b/robot-server/tests/runs/router/test_base_router.py index bb7f723138f..ee9b291cc4a 100644 --- a/robot-server/tests/runs/router/test_base_router.py +++ b/robot-server/tests/runs/router/test_base_router.py @@ -25,7 +25,6 @@ from robot_server.data_files.models import DataFileSource from robot_server.errors.error_responses import ApiError -from robot_server.runs.error_recovery_models import ErrorRecoveryPolicy from robot_server.service.json_api import ( RequestModel, SimpleBody, @@ -67,7 +66,6 @@ get_runs, remove_run, update_run, - put_error_recovery_policy, get_run_commands_error, get_current_state, CurrentStateLinks, @@ -709,44 +707,6 @@ async def test_update_to_current_missing( assert exc_info.value.content["errors"][0]["id"] == "RunNotFound" -async def test_create_policies( - decoy: Decoy, mock_run_data_manager: RunDataManager -) -> None: - """It should call RunDataManager create run policies.""" - policies = decoy.mock(cls=ErrorRecoveryPolicy) - await put_error_recovery_policy( - runId="rud-id", - request_body=RequestModel(data=policies), - run_data_manager=mock_run_data_manager, - ) - decoy.verify( - mock_run_data_manager.set_error_recovery_rules( - run_id="rud-id", rules=policies.policyRules - ) - ) - - -async def test_create_policies_raises_not_active_run( - decoy: Decoy, mock_run_data_manager: RunDataManager -) -> None: - """It should raise that the run is not current.""" - policies = decoy.mock(cls=ErrorRecoveryPolicy) - decoy.when( - mock_run_data_manager.set_error_recovery_rules( - run_id="rud-id", rules=policies.policyRules - ) - ).then_raise(RunNotCurrentError()) - with pytest.raises(ApiError) as exc_info: - await put_error_recovery_policy( - runId="rud-id", - request_body=RequestModel(data=policies), - run_data_manager=mock_run_data_manager, - ) - - assert exc_info.value.status_code == 409 - assert exc_info.value.content["errors"][0]["id"] == "RunStopped" - - async def test_get_run_commands_errors( decoy: Decoy, mock_run_data_manager: RunDataManager ) -> None: diff --git a/robot-server/tests/runs/router/test_error_recovery_policy_router.py b/robot-server/tests/runs/router/test_error_recovery_policy_router.py new file mode 100644 index 00000000000..efdf4d15f53 --- /dev/null +++ b/robot-server/tests/runs/router/test_error_recovery_policy_router.py @@ -0,0 +1,90 @@ +# noqa: D100 + +import pytest +from decoy import Decoy + +from robot_server.errors.error_responses import ApiError +from robot_server.runs import error_recovery_models as er_models +from robot_server.runs.router.error_recovery_policy_router import ( + get_error_recovery_policy, + put_error_recovery_policy, +) +from robot_server.runs.run_data_manager import RunDataManager, RunNotCurrentError +from robot_server.service.json_api.request import RequestModel + + +async def test_put(decoy: Decoy, mock_run_data_manager: RunDataManager) -> None: + """It should call RunDataManager create run policies.""" + policies = decoy.mock(cls=er_models.ErrorRecoveryPolicy) + await put_error_recovery_policy( + runId="run-id", + request_body=RequestModel(data=policies), + run_data_manager=mock_run_data_manager, + ) + decoy.verify( + mock_run_data_manager.set_error_recovery_rules( + run_id="run-id", rules=policies.policyRules + ) + ) + + +async def test_put_raises_not_active_run( + decoy: Decoy, mock_run_data_manager: RunDataManager +) -> None: + """It should raise that the run is not current.""" + policies = decoy.mock(cls=er_models.ErrorRecoveryPolicy) + decoy.when( + mock_run_data_manager.set_error_recovery_rules( + run_id="run-id", rules=policies.policyRules + ) + ).then_raise(RunNotCurrentError()) + with pytest.raises(ApiError) as exc_info: + await put_error_recovery_policy( + runId="run-id", + request_body=RequestModel(data=policies), + run_data_manager=mock_run_data_manager, + ) + + assert exc_info.value.status_code == 409 + assert exc_info.value.content["errors"][0]["id"] == "RunStopped" + + +async def test_get(decoy: Decoy, mock_run_data_manager: RunDataManager) -> None: + """It should call RunDataManager create run policies.""" + rule = er_models.ErrorRecoveryRule( + matchCriteria=er_models.MatchCriteria( + command=er_models.CommandMatcher( + commandType="commandType", + error=er_models.ErrorMatcher(errorType="errorType"), + ) + ), + ifMatch=er_models.ReactionIfMatch.WAIT_FOR_RECOVERY, + ) + + decoy.when(mock_run_data_manager.get_error_recovery_rules("run-id")).then_return( + [rule] + ) + result = await get_error_recovery_policy( + runId="run-id", + run_data_manager=mock_run_data_manager, + ) + assert result.status_code == 200 + assert result.content.data == er_models.ErrorRecoveryPolicy(policyRules=[rule]) + + +async def test_get_raises_not_active_run( + decoy: Decoy, mock_run_data_manager: RunDataManager +) -> None: + """It should raise that the run is not current.""" + decoy.when( + mock_run_data_manager.get_error_recovery_rules(run_id="run-id") + ).then_raise(RunNotCurrentError()) + + with pytest.raises(ApiError) as exc_info: + await get_error_recovery_policy( + runId="run-id", + run_data_manager=mock_run_data_manager, + ) + + assert exc_info.value.status_code == 409 + assert exc_info.value.content["errors"][0]["id"] == "RunStopped" diff --git a/robot-server/tests/runs/test_run_data_manager.py b/robot-server/tests/runs/test_run_data_manager.py index a26baacadbf..a78a7585c52 100644 --- a/robot-server/tests/runs/test_run_data_manager.py +++ b/robot-server/tests/runs/test_run_data_manager.py @@ -1254,7 +1254,7 @@ async def test_get_current_run_labware_definition( ] -async def test_create_policies_raises_run_not_current( +async def test_set_error_recovery_rules_raises_run_not_current( decoy: Decoy, mock_run_orchestrator_store: RunOrchestratorStore, subject: RunDataManager, @@ -1269,7 +1269,7 @@ async def test_create_policies_raises_run_not_current( ) -async def test_create_policies_translates_and_calls_orchestrator( +async def test_set_error_recovery_rules_translates_and_calls_orchestrator( decoy: Decoy, mock_run_orchestrator_store: RunOrchestratorStore, mock_error_recovery_setting_store: ErrorRecoverySettingStore, @@ -1296,6 +1296,34 @@ async def test_create_policies_translates_and_calls_orchestrator( ) +async def test_get_error_recovery_rules( + decoy: Decoy, + mock_run_orchestrator_store: RunOrchestratorStore, + subject: RunDataManager, +) -> None: + """It should return the current run's previously-set list of error recovery rules.""" + # Before there has been any current run, it should raise an exception. + with pytest.raises(RunNotCurrentError): + subject.get_error_recovery_rules(run_id="whatever") + + # While there is a current run, it should return its list of rules. + decoy.when(mock_run_orchestrator_store.current_run_id).then_return( + sentinel.current_run_id + ) + subject.set_error_recovery_rules( + run_id=sentinel.current_run_id, rules=sentinel.input_rules + ) + assert ( + subject.get_error_recovery_rules(run_id=sentinel.current_run_id) + == sentinel.input_rules + ) + + # When the run stops being current, it should go back to raising. + decoy.when(mock_run_orchestrator_store.current_run_id).then_return(None) + with pytest.raises(RunNotCurrentError): + subject.get_error_recovery_rules(run_id="whatever") + + def test_get_nozzle_map_current_run( decoy: Decoy, mock_run_orchestrator_store: RunOrchestratorStore, diff --git a/shared-data/python/opentrons_shared_data/pipette/mutable_configurations.py b/shared-data/python/opentrons_shared_data/pipette/mutable_configurations.py index 06b31215b65..7e1beb5dd35 100644 --- a/shared-data/python/opentrons_shared_data/pipette/mutable_configurations.py +++ b/shared-data/python/opentrons_shared_data/pipette/mutable_configurations.py @@ -41,7 +41,7 @@ LIQUID_CLASS = LiquidClasses.default -def _edit_non_quirk( +def _edit_non_quirk( # noqa: C901 mutable_config_key: str, new_mutable_value: MutableConfig, base_dict: Dict[str, Any] ) -> None: def _do_edit_non_quirk( @@ -58,6 +58,9 @@ def _do_edit_non_quirk( elif thiskey == "##EACHTIPTYPE##": for key in existing.keys(): _do_edit_non_quirk(new_value, existing[key], restkeys) + elif thiskey == "##EACHTIP##": + for key in existing.keys(): + _do_edit_non_quirk(new_value, existing[key], restkeys) else: _do_edit_non_quirk(new_value, existing[thiskey], restkeys) else: diff --git a/shared-data/python/tests/pipette/test_mutable_configurations.py b/shared-data/python/tests/pipette/test_mutable_configurations.py index 40059f6bfe6..38920c473e8 100644 --- a/shared-data/python/tests/pipette/test_mutable_configurations.py +++ b/shared-data/python/tests/pipette/test_mutable_configurations.py @@ -1,4 +1,5 @@ import json +import logging import os from pathlib import Path from typing import Dict, Any, cast, Union, Generator @@ -68,9 +69,8 @@ def test_load_old_overrides_regression( "type": "float", "default": 0.1, } - json.dump( - TMPFILE_DATA, open(override_configuration_path / "P20SV222021040709.json", "w") - ) + with open(override_configuration_path / "P20SV222021040709.json", "w") as f: + json.dump(TMPFILE_DATA, f) configs = mutable_configurations.load_with_mutable_configurations( pipette_definition.PipetteModelVersionType( pipette_type=types.PipetteModelType.p20, @@ -305,3 +305,159 @@ def test_build_mutable_config_using_old_units() -> None: assert ( types.MutableConfig.build(**old_units_config, name="dropTipSpeed") is not None # type: ignore ) + + +@pytest.mark.parametrize( + ("filename", "type", "channels", "version", "file_contents"), + # From https://opentrons.atlassian.net/browse/RQA-3676. + # These could probably be pared down. + [ + ( + "P20MV202020121412.json", + types.PipetteModelType.p20, + types.PipetteChannelType.EIGHT_CHANNEL, + types.PipetteVersionType(2, 0), + '{"model": "p20_multi_v2.0"}', + ), + ( + "P3HSV1318071638.json", + types.PipetteModelType.p300, + types.PipetteChannelType.SINGLE_CHANNEL, + types.PipetteVersionType(1, 3), + '{"dropTipShake": true, "model": "p300_single_v1.3", "quirks": {"dropTipShake": true}, "top": {"value": 30.0, "default": 19.5, "units": "mm", "type": "float", "min": -20, "max": 30}, "pickUpPresses": {"value": 3.0, "default": 3, "units": "presses", "type": "int", "min": 0, "max": 15}}', + ), + ( + "P3HMV212021040004.json", + types.PipetteModelType.p300, + types.PipetteChannelType.EIGHT_CHANNEL, + types.PipetteVersionType(2, 1), + '{"needsUnstick": true, "model": "p300_multi_v2.1"}', + ), + ( + "P20SV202020032604.json", + types.PipetteModelType.p20, + types.PipetteChannelType.SINGLE_CHANNEL, + types.PipetteVersionType(2, 0), + '{"model": "p20_single_v2.0"}', + ), + ( + "P1KSV202019072441.json", + types.PipetteModelType.p1000, + types.PipetteChannelType.SINGLE_CHANNEL, + types.PipetteVersionType(2, 0), + '{"pickupTipShake": true, "model": "p1000_single_v2.0", "quirks": {"pickupTipShake": true}}', + ), + ( + "P3HMV202021011105.json", + types.PipetteModelType.p300, + types.PipetteChannelType.EIGHT_CHANNEL, + types.PipetteVersionType(2, 0), + '{"needsUnstick": true, "model": "p300_multi_v2.0"}', + ), + ( + "P20SV202019072527.json", + types.PipetteModelType.p20, + types.PipetteChannelType.SINGLE_CHANNEL, + types.PipetteVersionType(2, 0), + '{"model": "p20_single_v2.0"}', + ), + ( + "P3HSV202021042602.json", + types.PipetteModelType.p300, + types.PipetteChannelType.SINGLE_CHANNEL, + types.PipetteVersionType(2, 0), + '{"model": "p300_single_v2.0"}', + ), + ( + "P20SV222021030914.json", + types.PipetteModelType.p20, + types.PipetteChannelType.SINGLE_CHANNEL, + types.PipetteVersionType(2, 2), + '{"model": "p20_single_v2.2", "quirks": {}, "pickUpPresses": {"value": 3.0, "default": 1, "units": "presses", "type": "int", "min": 0, "max": 15}}', + ), + ( + "P3HMV202020040801.json", + types.PipetteModelType.p300, + types.PipetteChannelType.EIGHT_CHANNEL, + types.PipetteVersionType(2, 0), + '{"needsUnstick": true, "model": "p300_multi_v2.0", "quirks": {"needsUnstick": true}, "top": {"value": 20.0, "default": 19.5, "units": "mm", "type": "float", "min": -20, "max": 30}, "bottom": {"value": -14.0, "default": -14.5, "units": "mm", "type": "float", "min": -20, "max": 30}, "blowout": {"value": -18.5, "default": -19.0, "units": "mm", "type": "float", "min": -20, "max": 30}, "dropTip": {"value": -20.0, "default": -33.4, "units": "mm", "type": "float", "min": -20, "max": 30}, "pickUpCurrent": {"value": 0.9, "default": 0.8, "units": "amps", "type": "float", "min": 0.1, "max": 2.0}, "pickUpDistance": {"value": 10.0, "default": 11.0, "units": "mm", "type": "float", "min": 0, "max": 10}, "pickUpIncrement": {"value": 0.1, "default": 0.0, "units": "mm", "type": "int", "min": 0, "max": 10}, "pickUpPresses": {"value": 4.0, "default": 1, "units": "presses", "type": "int", "min": 0, "max": 15}, "pickUpSpeed": {"value": 11.0, "default": 10.0, "units": "mm/s", "type": "float", "min": 0.01, "max": 30}, "plungerCurrent": {"value": 1.1, "default": 1.0, "units": "amps", "type": "float", "min": 0.1, "max": 2.0}, "dropTipCurrent": {"value": 1.22, "default": 1.25, "units": "amps", "type": "float", "min": 0.1, "max": 2.0}, "dropTipSpeed": {"value": 8.0, "default": 7.5, "units": "mm/s", "type": "float", "min": 0.01, "max": 30}, "tipLength": {"value": 52.0, "default": 51.0, "units": "mm", "type": "float", "min": 0, "max": 100}}', + ), + ( + "P3HMV212021040002.json", + types.PipetteModelType.p300, + types.PipetteChannelType.EIGHT_CHANNEL, + types.PipetteVersionType(2, 1), + '{"needsUnstick": true, "model": "p300_multi_v2.1"}', + ), + ( + "P3HMV1318072625.json", + types.PipetteModelType.p300, + types.PipetteChannelType.EIGHT_CHANNEL, + types.PipetteVersionType(1, 3), + '{"dropTipShake": true, "model": "p300_multi_v1.3", "quirks": {"dropTipShake": true}, "pickUpPresses": {"value": 4.0, "default": 3, "units": "presses", "type": "int", "min": 0, "max": 15}}', + ), + ( + "P50MV1519091757.json", + types.PipetteModelType.p50, + types.PipetteChannelType.EIGHT_CHANNEL, + types.PipetteVersionType(1, 5), + '{"dropTipShake": true, "doubleDropTip": true, "model": "p50_multi_v1.5", "quirks": {"doubleDropTip": true, "dropTipShake": true}}', + ), + ( + "P3HSV202019072224.json", + types.PipetteModelType.p300, + types.PipetteChannelType.SINGLE_CHANNEL, + types.PipetteVersionType(2, 0), + '{"model": "p300_single_v2.0", "quirks": {}}', + ), + ( + "P20MV202019112708.json", + types.PipetteModelType.p20, + types.PipetteChannelType.EIGHT_CHANNEL, + types.PipetteVersionType(2, 0), + '{"model": "p20_multi_v2.0", "quirks": {}}', + ), + ( + "P3HSV202021031503.json", + types.PipetteModelType.p300, + types.PipetteChannelType.SINGLE_CHANNEL, + types.PipetteVersionType(2, 1), + '{"model": "p300_single_v2.1"}', + ), + ( + "P1KSV202020060206.json", + types.PipetteModelType.p1000, + types.PipetteChannelType.SINGLE_CHANNEL, + types.PipetteVersionType(2, 0), + '{"pickupTipShake": true, "model": "p1000_single_v2.0"}', + ), + ( + "P3HMV202021010906.json", + types.PipetteModelType.p300, + types.PipetteChannelType.EIGHT_CHANNEL, + types.PipetteVersionType(2, 0), + '{"needsUnstick": true, "model": "p300_multi_v2.0"}', + ), + ], +) +def test_loading_does_not_log_warnings( + filename: str, + type: types.PipetteModelType, + channels: types.PipetteChannelType, + version: types.PipetteVersionType, + file_contents: str, + caplog: pytest.LogCaptureFixture, + override_configuration_path: Path, +) -> None: + """Make sure load_with_mutable_configurations() doesn't log any exceptions. + + load_with_mutable_configurations() suppresses and logs internal exceptions to + protect its caller, but those are still bugs, and we still want tests to catch them. + """ + model = pipette_definition.PipetteModelVersionType(type, channels, version) + (override_configuration_path / filename).write_text(file_contents) + with caplog.at_level(logging.WARNING): + mutable_configurations.load_with_mutable_configurations( + model, override_configuration_path, Path(filename).stem + ) + assert len(caplog.records) == 0