From f4ee5f59dc24ec6eccb306cc3d24db09cefa7e17 Mon Sep 17 00:00:00 2001 From: Josh McVey Date: Tue, 21 May 2024 14:14:59 -0500 Subject: [PATCH 01/17] fix(docker): need to specify platform (#15236) ### Forgot to specify the platform explicitly. > When it was built on a mac, we got the wrong platform. --- opentrons-ai-server/Dockerfile | 2 +- opentrons-ai-server/deploy.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/opentrons-ai-server/Dockerfile b/opentrons-ai-server/Dockerfile index 5c3a5ec5f36..8a4ee3b944d 100644 --- a/opentrons-ai-server/Dockerfile +++ b/opentrons-ai-server/Dockerfile @@ -1,4 +1,4 @@ -FROM public.ecr.aws/lambda/python:3.12 +FROM --platform=linux/amd64 public.ecr.aws/lambda/python:3.12 COPY --from=public.ecr.aws/datadog/lambda-extension:57 /opt/. /opt/ diff --git a/opentrons-ai-server/deploy.py b/opentrons-ai-server/deploy.py index c33d249f4d7..21190a85e90 100644 --- a/opentrons-ai-server/deploy.py +++ b/opentrons-ai-server/deploy.py @@ -37,7 +37,7 @@ class BaseDeploymentConfig: @dataclass(frozen=True) class CrtDeploymentConfig(BaseDeploymentConfig): ECR_REPOSITORY: str = "crt-ecr-repo" - ECR_URL: str = f"{get_aws_account_id}.dkr.ecr.{get_aws_region()}.amazonaws.com" + ECR_URL: str = f"{get_aws_account_id()}.dkr.ecr.{get_aws_region()}.amazonaws.com" FUNCTION_NAME: str = "crt-api-function" IMAGE_NAME: str = "crt-ai-server" From 4fd72b69f76a2980260039abbf2a20c820a553eb Mon Sep 17 00:00:00 2001 From: Nick Diehl <47604184+ncdiehl11@users.noreply.github.com> Date: Tue, 21 May 2024 16:08:23 -0400 Subject: [PATCH 02/17] fix(protocol-designer): fix diffing function in makeTimelineMiddleware (#15235) Closes RQA-2752 Undefined object values were resulting in `false` returns for `substepsNeedsRecompute`, in turn resulting in not propogating up errors from moving labware conflicts. This PR uses deep object equality checks with lodash isEqual to properly return true if our robot timeline needs recomputing. --- .../src/timelineMiddleware/makeTimelineMiddleware.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/protocol-designer/src/timelineMiddleware/makeTimelineMiddleware.ts b/protocol-designer/src/timelineMiddleware/makeTimelineMiddleware.ts index 2b6e6f2bc25..894469d0a70 100644 --- a/protocol-designer/src/timelineMiddleware/makeTimelineMiddleware.ts +++ b/protocol-designer/src/timelineMiddleware/makeTimelineMiddleware.ts @@ -1,3 +1,4 @@ +import isEqual from 'lodash/isEqual' import { getArgsAndErrorsByStepId, getOrderedStepIds, @@ -21,7 +22,7 @@ const hasChanged = ( ): boolean => Object.keys(nextValues).some( (selectorKey: string) => - nextValues[selectorKey] !== memoizedValues?.[selectorKey] + !isEqual(nextValues[selectorKey], memoizedValues?.[selectorKey]) ) const getTimelineArgs = (state: BaseState): GenerateRobotStateTimelineArgs => ({ From a6f64a4c77ec48cb0bdd48268353e5013a980a5f Mon Sep 17 00:00:00 2001 From: Nick Diehl <47604184+ncdiehl11@users.noreply.github.com> Date: Tue, 21 May 2024 16:09:08 -0400 Subject: [PATCH 03/17] fix(protocol-designer): set default z value for TipPositionModal (#15237) Closes RQA-2750 If selecting a custom offset for TipPositionModal on a transfer step, the Z field should populate with the default offset from bottom rather than empty. --- .../StepEditForm/fields/TipPositionField/TipPositionModal.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocol-designer/src/components/StepEditForm/fields/TipPositionField/TipPositionModal.tsx b/protocol-designer/src/components/StepEditForm/fields/TipPositionField/TipPositionModal.tsx index 297b2bdb9a0..f4df9bd2425 100644 --- a/protocol-designer/src/components/StepEditForm/fields/TipPositionField/TipPositionModal.tsx +++ b/protocol-designer/src/components/StepEditForm/fields/TipPositionField/TipPositionModal.tsx @@ -72,7 +72,7 @@ export const TipPositionModal = ( }) const [zValue, setZValue] = React.useState( - zSpec?.value == null ? null : String(zSpec?.value) + zSpec?.value == null ? String(defaultMmFromBottom) : String(zSpec?.value) ) const [yValue, setYValue] = React.useState( ySpec?.value == null ? null : String(ySpec?.value) From 99df21c58491543212410d9d8e1ac86f305c89f3 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Tue, 21 May 2024 17:12:25 -0400 Subject: [PATCH 04/17] feat(app): Add incompatible module notification modals (#15181) We meant to add this for the launch of the Flex, but it got cut for scope; in the time since, people connecting modules like first-generation temperature modules to Flexes that don't support them has been a pretty consistent thorn in the side of support. This PR implements the modals designed by design for telling the user when there's incompatible modules. This PR is pretty big and might be best viewed by commit, since it has both the backend and the frontend, and both the ODD and the desktop - and a couple ancillary support PRs that I'll rebase out of here as they get merged. Specifically, ## Overview 90dc748fbb - Python, adds data to module configs to say what kinds of deck they're compatible with, and adds code to the api and robot server to expose a bool about whether the pipette's compatible or not in the HTTP API 788e022bac - JS, adds the above to the api clients 2e49f6160c - JS, adds a blocking modal to the ODD that pops up whenever modules are attached per [figma](https://www.figma.com/file/0kuPeCi1t2Auu2GPMnqRb2/Primary%3A-Flex?type=design&node-id=19961-155919&mode=design&t=pkbLPA9150bwQQNw-4) c5021ca145 - JS, adds a modal that blocks interaction with a robot but _not_ the navbar or breadcrumbs whenever modules are attached per [figma](https://www.figma.com/design/Nx7ORMnfyJNP4FQYxN4e5x/Primary%3A-Desktop-App?node-id=2032-304206&t=iat6IH7YQD4pMijc-4) ## Notes and Review requests 90dc748fbb - I'm pretty sure these are the module compatibility rules, but we'll need to make sure. This will get exposed via HTTP as soon as the module is plugged in. Review requests: - [ ] bool should be correct for plugged-in modules basically as soon as they appear in the app The above, and 788e022bac - This is set up to support polling. Let me know if you think this should be notifications instead, will be another couple commits though. 2e49f6160c - ODD modal. Is this according to design, is there something I'm missing? Is the testing approach OK? You can test by editing `robot-server/simulator/test-flex.env` to specify a `ThermocyclerModuleV1` instead of a `ThermocyclerModuleV2` and run with `make -C robot-server dev-flex`, as well as actually plugging stuff in to a real robot. c5021ca145 - Desktop modal, basically same questions and same testing approach. Note that this is the first ish time we've added a modal that doesn't inhibit interaction with navbar or breadcrumbs. --- api-client/src/modules/__fixtures__/index.ts | 4 + api-client/src/modules/api-types.ts | 1 + app/src/App/DesktopApp.tsx | 29 +- app/src/App/OnDeviceDisplayApp.tsx | 2 + app/src/App/portal.tsx | 8 +- .../localization/en/incompatible_modules.json | 7 + app/src/assets/localization/en/index.ts | 2 + .../IncompatibleModuleDesktopModalBody.tsx | 92 +++++ .../IncompatibleModuleODDModalBody.tsx | 55 +++ .../IncompatibleModuleTakeover.tsx | 39 ++ .../IncompatibleModule/__fixtures__/index.ts | 154 +++++++ ...ncompatibleModuleDesktopModalBody.test.tsx | 54 +++ .../IncompatibleModuleODDModalBody.test.tsx | 45 ++ .../IncompatibleModuleTakeover.test.tsx | 94 +++++ .../incompatibleModuleFixtures.ts | 386 ++++++++++++++++++ .../hooks/__fixtures__/index.ts | 1 + .../useIncompatibleModulesAttached.test.tsx | 72 ++++ .../IncompatibleModule/hooks/index.ts | 1 + .../hooks/useIncompatibleModulesAttached.ts | 17 + .../organisms/IncompatibleModule/index.tsx | 1 + .../modules/module_data_mapper.py | 14 +- .../robot_server/modules/module_models.py | 3 + .../integration/test_modules.tavern.yaml | 99 ++++- .../tests/modules/test_module_data_mapper.py | 97 ++++- robot-server/tests/modules/test_router.py | 5 +- .../definitions/3/absorbanceReaderV1.json | 1 + .../definitions/3/heaterShakerModuleV1.json | 1 + .../module/definitions/3/magneticBlockV1.json | 1 + .../definitions/3/magneticModuleV1.json | 1 + .../definitions/3/magneticModuleV2.json | 1 + .../definitions/3/temperatureModuleV1.json | 1 + .../definitions/3/temperatureModuleV2.json | 1 + .../definitions/3/thermocyclerModuleV1.json | 1 + .../definitions/3/thermocyclerModuleV2.json | 1 + shared-data/module/schemas/3.json | 10 +- .../opentrons_shared_data/module/dev_types.py | 1 + 36 files changed, 1274 insertions(+), 28 deletions(-) create mode 100644 app/src/assets/localization/en/incompatible_modules.json create mode 100644 app/src/organisms/IncompatibleModule/IncompatibleModuleDesktopModalBody.tsx create mode 100644 app/src/organisms/IncompatibleModule/IncompatibleModuleODDModalBody.tsx create mode 100644 app/src/organisms/IncompatibleModule/IncompatibleModuleTakeover.tsx create mode 100644 app/src/organisms/IncompatibleModule/__fixtures__/index.ts create mode 100644 app/src/organisms/IncompatibleModule/__tests__/IncompatibleModuleDesktopModalBody.test.tsx create mode 100644 app/src/organisms/IncompatibleModule/__tests__/IncompatibleModuleODDModalBody.test.tsx create mode 100644 app/src/organisms/IncompatibleModule/__tests__/IncompatibleModuleTakeover.test.tsx create mode 100644 app/src/organisms/IncompatibleModule/hooks/__fixtures__/incompatibleModuleFixtures.ts create mode 100644 app/src/organisms/IncompatibleModule/hooks/__fixtures__/index.ts create mode 100644 app/src/organisms/IncompatibleModule/hooks/__tests__/useIncompatibleModulesAttached.test.tsx create mode 100644 app/src/organisms/IncompatibleModule/hooks/index.ts create mode 100644 app/src/organisms/IncompatibleModule/hooks/useIncompatibleModulesAttached.ts create mode 100644 app/src/organisms/IncompatibleModule/index.tsx diff --git a/api-client/src/modules/__fixtures__/index.ts b/api-client/src/modules/__fixtures__/index.ts index abba8731561..52bb9dc57f0 100644 --- a/api-client/src/modules/__fixtures__/index.ts +++ b/api-client/src/modules/__fixtures__/index.ts @@ -7,6 +7,7 @@ export const mockModulesResponse = [ hasAvailableUpdate: false, moduleType: 'thermocyclerModuleType', moduleModel: 'thermocyclerModuleV1', + compatibleWithRobot: true, data: { status: 'holding at target', currentTemperature: 3.0, @@ -31,6 +32,7 @@ export const mockModulesResponse = [ hasAvailableUpdate: false, moduleType: 'heaterShakerModuleType', moduleModel: 'heaterShakerModuleV1', + compatibleWithRobot: true, data: { status: 'idle', labwareLatchStatus: 'idle_unknown', @@ -55,6 +57,7 @@ export const mockModulesResponse = [ hasAvailableUpdate: false, moduleType: 'temperatureModuleType', moduleModel: 'temperatureModuleV1', + compatibleWithRobot: true, data: { status: 'holding at target', currentTemperature: 3.0, @@ -75,6 +78,7 @@ export const mockModulesResponse = [ hasAvailableUpdate: false, moduleType: 'magneticModuleType', moduleModel: 'magneticModuleV1', + compatibleWithRobot: true, data: { status: 'engaged', engaged: true, diff --git a/api-client/src/modules/api-types.ts b/api-client/src/modules/api-types.ts index 68c4f3f26e7..60203dba15e 100644 --- a/api-client/src/modules/api-types.ts +++ b/api-client/src/modules/api-types.ts @@ -36,6 +36,7 @@ export interface ApiBaseModule { firmwareVersion: string hasAvailableUpdate: boolean usbPort: PhysicalPort + compatibleWithRobot?: boolean moduleOffset?: ModuleOffset } diff --git a/app/src/App/DesktopApp.tsx b/app/src/App/DesktopApp.tsx index ffa50727da1..c4b8aa6d6cb 100644 --- a/app/src/App/DesktopApp.tsx +++ b/app/src/App/DesktopApp.tsx @@ -28,6 +28,7 @@ import { Labware } from '../pages/Labware' import { useSoftwareUpdatePoll } from './hooks' import { Navbar } from './Navbar' import { EstopTakeover, EmergencyStopContext } from '../organisms/EmergencyStop' +import { IncompatibleModuleTakeover } from '../organisms/IncompatibleModule' import { OPENTRONS_USB } from '../redux/discovery' import { appShellRequestor } from '../redux/shell/remote' import { useRobot, useIsFlex } from '../organisms/Devices/hooks' @@ -153,10 +154,6 @@ function RobotControlTakeover(): JSX.Element | null { const params = deviceRouteMatch?.params as DesktopRouteParams const robotName = params?.robotName const robot = useRobot(robotName) - const isFlex = useIsFlex(robotName) - - // E-stop is not supported on OT2 - if (!isFlex) return null if (deviceRouteMatch == null || robot == null || robotName == null) return null @@ -167,7 +164,29 @@ function RobotControlTakeover(): JSX.Element | null { hostname={robot.ip ?? null} requestor={robot?.ip === OPENTRONS_USB ? appShellRequestor : undefined} > - + + ) } + +interface TakeoverProps { + robotName: string +} + +function AllRobotsRobotControlTakeover({ + robotName, +}: TakeoverProps): JSX.Element | null { + return +} + +function FlexOnlyRobotControlTakeover({ + robotName, +}: TakeoverProps): JSX.Element | null { + // E-stop is not supported on OT2 + const isFlex = useIsFlex(robotName) + if (!isFlex) { + return null + } + return +} diff --git a/app/src/App/OnDeviceDisplayApp.tsx b/app/src/App/OnDeviceDisplayApp.tsx index 1459ff5071f..43f93bd425b 100644 --- a/app/src/App/OnDeviceDisplayApp.tsx +++ b/app/src/App/OnDeviceDisplayApp.tsx @@ -20,6 +20,7 @@ import { OnDeviceLocalizationProvider } from '../LocalizationProvider' import { ToasterOven } from '../organisms/ToasterOven' import { MaintenanceRunTakeover } from '../organisms/TakeoverModal' import { FirmwareUpdateTakeover } from '../organisms/FirmwareUpdateModal/FirmwareUpdateTakeover' +import { IncompatibleModuleTakeover } from '../organisms/IncompatibleModule' import { EstopTakeover } from '../organisms/EmergencyStop' import { ConnectViaEthernet } from '../pages/ConnectViaEthernet' import { ConnectViaUSB } from '../pages/ConnectViaUSB' @@ -179,6 +180,7 @@ export const OnDeviceDisplayApp = (): JSX.Element => { ) : ( <> + diff --git a/app/src/App/portal.tsx b/app/src/App/portal.tsx index 62f5d79fcf2..346d5842d81 100644 --- a/app/src/App/portal.tsx +++ b/app/src/App/portal.tsx @@ -1,8 +1,8 @@ import * as React from 'react' import { Box } from '@opentrons/components' -const TOP_PORTAL_ID = '__otAppTopPortalRoot' -const MODAL_PORTAL_ID = '__otAppModalPortalRoot' +export const TOP_PORTAL_ID = '__otAppTopPortalRoot' +export const MODAL_PORTAL_ID = '__otAppModalPortalRoot' export function getTopPortalEl(): HTMLElement { return global.document.getElementById(TOP_PORTAL_ID) ?? global.document.body } @@ -11,9 +11,9 @@ export function getModalPortalEl(): HTMLElement { } export function PortalRoot(): JSX.Element { - return + return } export function TopPortalRoot(): JSX.Element { - return + return } diff --git a/app/src/assets/localization/en/incompatible_modules.json b/app/src/assets/localization/en/incompatible_modules.json new file mode 100644 index 00000000000..d9b1a231f0c --- /dev/null +++ b/app/src/assets/localization/en/incompatible_modules.json @@ -0,0 +1,7 @@ +{ + "incompatible_modules_attached": "incompatible module detected", + "remove_before_running_protocol": "Remove the following hardware before running a protocol:", + "needs_your_assistance": "{{robot_name}} needs your assistance", + "remove_before_using": "You must remove incompatible modules before using this robot.", + "is_not_compatible": "{{module_name}} is not compatible with the {{robot_type}}" +} diff --git a/app/src/assets/localization/en/index.ts b/app/src/assets/localization/en/index.ts index 51acf92db53..aef6c301f3e 100644 --- a/app/src/assets/localization/en/index.ts +++ b/app/src/assets/localization/en/index.ts @@ -27,6 +27,7 @@ import robot_controls from './robot_controls.json' import run_details from './run_details.json' import top_navigation from './top_navigation.json' import error_recovery from './error_recovery.json' +import incompatible_modules from './incompatible_modules.json' export const en = { shared, @@ -58,4 +59,5 @@ export const en = { run_details, top_navigation, error_recovery, + incompatible_modules, } diff --git a/app/src/organisms/IncompatibleModule/IncompatibleModuleDesktopModalBody.tsx b/app/src/organisms/IncompatibleModule/IncompatibleModuleDesktopModalBody.tsx new file mode 100644 index 00000000000..ac4c8c5993d --- /dev/null +++ b/app/src/organisms/IncompatibleModule/IncompatibleModuleDesktopModalBody.tsx @@ -0,0 +1,92 @@ +import * as React from 'react' +import { useTranslation, Trans } from 'react-i18next' +import { + DIRECTION_COLUMN, + DIRECTION_ROW, + ALIGN_CENTER, + JUSTIFY_FLEX_START, + Flex, + SPACING, + StyledText, + TYPOGRAPHY, + OVERFLOW_SCROLL, + Icon, + COLORS, +} from '@opentrons/components' +import { getModuleDisplayName } from '@opentrons/shared-data' +import type { AttachedModule } from '@opentrons/api-client' +import { useIsFlex } from '../Devices/hooks' +import { InterventionModal } from '../../molecules/InterventionModal' +export interface IncompatibleModuleDesktopModalBodyProps { + modules: AttachedModule[] + robotName: string +} + +export function IncompatibleModuleDesktopModalBody({ + modules, + robotName, +}: IncompatibleModuleDesktopModalBodyProps): JSX.Element { + const { t } = useTranslation('incompatible_modules') + const isFlex = useIsFlex(robotName) + const displayName = isFlex ? 'Flex' : 'OT-2' + return ( + + } + type="error" + > + + + {modules.map(module => ( +
  • + + + + + + +
    • + ))} +     + + + +     +     + ) +} diff --git a/app/src/organisms/IncompatibleModule/IncompatibleModuleODDModalBody.tsx b/app/src/organisms/IncompatibleModule/IncompatibleModuleODDModalBody.tsx new file mode 100644 index 00000000000..fb4981c0c71 --- /dev/null +++ b/app/src/organisms/IncompatibleModule/IncompatibleModuleODDModalBody.tsx @@ -0,0 +1,55 @@ +import * as React from 'react' +import { useTranslation, Trans } from 'react-i18next' +import capitalize from 'lodash/capitalize' +import { + DIRECTION_COLUMN, + Flex, + SPACING, + StyledText, + TYPOGRAPHY, + OVERFLOW_SCROLL, +} from '@opentrons/components' +import { getModuleDisplayName } from '@opentrons/shared-data' +import type { AttachedModule } from '@opentrons/api-client' +import { Modal } from '../../molecules/Modal' +import { ListItem } from '../../atoms/ListItem' +import type { ModalHeaderBaseProps } from '../../molecules/Modal/types' +export interface IncompatibleModuleODDModalBodyProps { + modules: AttachedModule[] +} + +export function IncompatibleModuleODDModalBody({ + modules, +}: IncompatibleModuleODDModalBodyProps): JSX.Element { + const { t } = useTranslation('incompatible_modules') + const incompatibleModuleHeader: ModalHeaderBaseProps = { + title: capitalize(t('incompatible_modules_attached')), + } + return ( + + + + + + + {modules.map(module => ( + + + {getModuleDisplayName(module.moduleModel)} + + + ))} + + + + ) +} diff --git a/app/src/organisms/IncompatibleModule/IncompatibleModuleTakeover.tsx b/app/src/organisms/IncompatibleModule/IncompatibleModuleTakeover.tsx new file mode 100644 index 00000000000..3be98ad14fc --- /dev/null +++ b/app/src/organisms/IncompatibleModule/IncompatibleModuleTakeover.tsx @@ -0,0 +1,39 @@ +import * as React from 'react' +import { createPortal } from 'react-dom' +import { IncompatibleModuleODDModalBody } from './IncompatibleModuleODDModalBody' +import { IncompatibleModuleDesktopModalBody } from './IncompatibleModuleDesktopModalBody' +import { getTopPortalEl, getModalPortalEl } from '../../App/portal' +import { useIncompatibleModulesAttached } from './hooks' + +const POLL_INTERVAL_MS = 5000 + +export interface IncompatibleModuleTakeoverProps { + isOnDevice: boolean + robotName?: string +} + +export function IncompatibleModuleTakeover({ + isOnDevice, + robotName, +}: IncompatibleModuleTakeoverProps): JSX.Element | null { + const incompatibleModules = useIncompatibleModulesAttached({ + refetchInterval: POLL_INTERVAL_MS, + }) + if (incompatibleModules.length === 0) { + return null + } + if (isOnDevice) { + return createPortal( + , + getTopPortalEl() + ) + } else { + return createPortal( + , + getModalPortalEl() + ) + } +} diff --git a/app/src/organisms/IncompatibleModule/__fixtures__/index.ts b/app/src/organisms/IncompatibleModule/__fixtures__/index.ts new file mode 100644 index 00000000000..35399d22450 --- /dev/null +++ b/app/src/organisms/IncompatibleModule/__fixtures__/index.ts @@ -0,0 +1,154 @@ +export const oneIncompatibleModule = [ + { + id: '3feb840a3fa2dac2409b977f1e330f54f50e6231', + serialNumber: 'dummySerialTC', + firmwareVersion: 'dummyVersionTC', + hardwareRevision: 'dummyModelTC', + hasAvailableUpdate: false, + moduleType: 'thermocyclerModuleType', + moduleModel: 'thermocyclerModuleV1', + compatibleWithRobot: false, + data: { + status: 'holding at target', + currentTemperature: 3.0, + targetTemperature: 3.0, + lidStatus: 'open', + lidTemperature: 4.0, + lidTargetTemperature: 4.0, + holdTime: 121.0, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, +] +export const manyIncompatibleModules = [ + { + id: '3feb840a3fa2dac2409b977f1e330f54f50e6231', + serialNumber: 'dummySerialTC', + firmwareVersion: 'dummyVersionTC', + hardwareRevision: 'dummyModelTC', + hasAvailableUpdate: false, + moduleType: 'thermocyclerModuleType', + moduleModel: 'thermocyclerModuleV1', + compatibleWithRobot: false, + data: { + status: 'holding at target', + currentTemperature: 3.0, + targetTemperature: 3.0, + lidStatus: 'open', + lidTemperature: 4.0, + lidTargetTemperature: 4.0, + holdTime: 121.0, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, + { + id: 'aojfhkalshdaoahosifhoaisdada', + serialNumber: 'dummySerialTC', + firmwareVersion: 'dummyVersionTC', + hardwareRevision: 'dummyModelTC', + hasAvailableUpdate: false, + moduleType: 'thermocyclerModuleType', + moduleModel: 'thermocyclerModuleV1', + compatibleWithRobot: false, + data: { + status: 'holding at target', + currentTemperature: 3.0, + targetTemperature: 3.0, + lidStatus: 'open', + lidTemperature: 4.0, + lidTargetTemperature: 4.0, + holdTime: 121.0, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, + { + id: 'asojhfaohsoihfjaoisodaalallala', + serialNumber: 'dummySerialTC', + firmwareVersion: 'dummyVersionTC', + hardwareRevision: 'dummyModelTC', + hasAvailableUpdate: false, + moduleType: 'thermocyclerModuleType', + moduleModel: 'thermocyclerModuleV1', + compatibleWithRobot: false, + data: { + status: 'holding at target', + currentTemperature: 3.0, + targetTemperature: 3.0, + lidStatus: 'open', + lidTemperature: 4.0, + lidTargetTemperature: 4.0, + holdTime: 121.0, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, + { + id: 'sfaoisdfolasda09sd09aaaaaaaaaa', + serialNumber: 'dummySerialTC', + firmwareVersion: 'dummyVersionTC', + hardwareRevision: 'dummyModelTC', + hasAvailableUpdate: false, + moduleType: 'thermocyclerModuleType', + moduleModel: 'thermocyclerModuleV1', + compatibleWithRobot: false, + data: { + status: 'holding at target', + currentTemperature: 3.0, + targetTemperature: 3.0, + lidStatus: 'open', + lidTemperature: 4.0, + lidTargetTemperature: 4.0, + holdTime: 121.0, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, + { + id: 'oasihfa980109109dm011', + serialNumber: 'dummySerialTC', + firmwareVersion: 'dummyVersionTC', + hardwareRevision: 'dummyModelTC', + hasAvailableUpdate: false, + moduleType: 'thermocyclerModuleType', + moduleModel: 'thermocyclerModuleV1', + compatibleWithRobot: false, + data: { + status: 'holding at target', + currentTemperature: 3.0, + targetTemperature: 3.0, + lidStatus: 'open', + lidTemperature: 4.0, + lidTargetTemperature: 4.0, + holdTime: 121.0, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, +] diff --git a/app/src/organisms/IncompatibleModule/__tests__/IncompatibleModuleDesktopModalBody.test.tsx b/app/src/organisms/IncompatibleModule/__tests__/IncompatibleModuleDesktopModalBody.test.tsx new file mode 100644 index 00000000000..b3d7fc7bbf3 --- /dev/null +++ b/app/src/organisms/IncompatibleModule/__tests__/IncompatibleModuleDesktopModalBody.test.tsx @@ -0,0 +1,54 @@ +import React from 'react' +import { screen } from '@testing-library/react' +import { describe, it, beforeEach, vi } from 'vitest' +import { when } from 'vitest-when' +import '@testing-library/jest-dom/vitest' +import { renderWithProviders } from '../../../__testing-utils__' +import { i18n } from '../../../i18n' +import { IncompatibleModuleDesktopModalBody } from '../IncompatibleModuleDesktopModalBody' +import { useIsFlex } from '../../Devices/hooks' +import * as Fixtures from '../__fixtures__' + +vi.mock('../../Devices/hooks') + +const getRenderer = (isFlex: boolean) => { + when(useIsFlex).calledWith('otie').thenReturn(isFlex) + return ( + props: React.ComponentProps + ) => { + return renderWithProviders( + , + { + i18nInstance: i18n, + } + )[0] + } +} + +describe('IncompatibleModuleDesktopModalBody', () => { + let props: React.ComponentProps + beforeEach(() => { + props = { + modules: [], + robotName: 'otie', + } + }) + + it('should render i18nd footer text', () => { + props = { ...props, modules: Fixtures.oneIncompatibleModule as any } + getRenderer(true)(props) + screen.getByText( + 'You must remove incompatible modules before using this robot.' + ) + screen.getByText('otie needs your assistance') + }) + ;['Flex', 'OT-2'].forEach(robotKind => + it(`should render a module card that says ${robotKind}`, () => { + props = { ...props, modules: Fixtures.oneIncompatibleModule as any } + getRenderer(robotKind === 'Flex')(props) + screen.getByText( + `Thermocycler Module GEN1 is not compatible with the ${robotKind}` + ) + }) + ) +}) diff --git a/app/src/organisms/IncompatibleModule/__tests__/IncompatibleModuleODDModalBody.test.tsx b/app/src/organisms/IncompatibleModule/__tests__/IncompatibleModuleODDModalBody.test.tsx new file mode 100644 index 00000000000..ce63b26ed88 --- /dev/null +++ b/app/src/organisms/IncompatibleModule/__tests__/IncompatibleModuleODDModalBody.test.tsx @@ -0,0 +1,45 @@ +import React from 'react' +import { screen } from '@testing-library/react' +import { describe, it, beforeEach, expect } from 'vitest' +import '@testing-library/jest-dom/vitest' +import { renderWithProviders } from '../../../__testing-utils__' +import { i18n } from '../../../i18n' +import { IncompatibleModuleODDModalBody } from '../IncompatibleModuleODDModalBody' +import * as Fixtures from '../__fixtures__' + +const render = ( + props: React.ComponentProps +) => { + return renderWithProviders(, { + i18nInstance: i18n, + })[0] +} + +describe('IncompatibleModuleODDModalBody', () => { + let props: React.ComponentProps + beforeEach(() => { + props = { + modules: [], + } + }) + + it('should render i18nd header text', () => { + props = { ...props, modules: Fixtures.oneIncompatibleModule as any } + render(props) + screen.getByText('Incompatible module detected') + screen.getByText('Remove the following hardware before running a protocol:') + }) + + it('should render a module card', () => { + props = { ...props, modules: Fixtures.oneIncompatibleModule as any } + render(props) + screen.getByText('Thermocycler Module GEN1') + }) + + it('should overflow via scroll', () => { + props = { ...props, modules: Fixtures.manyIncompatibleModules as any } + render(props) + const labels = screen.getAllByText('Thermocycler Module GEN1') + expect(labels).toHaveLength(Fixtures.manyIncompatibleModules.length) + }) +}) diff --git a/app/src/organisms/IncompatibleModule/__tests__/IncompatibleModuleTakeover.test.tsx b/app/src/organisms/IncompatibleModule/__tests__/IncompatibleModuleTakeover.test.tsx new file mode 100644 index 00000000000..d5c31e5cf3a --- /dev/null +++ b/app/src/organisms/IncompatibleModule/__tests__/IncompatibleModuleTakeover.test.tsx @@ -0,0 +1,94 @@ +import React from 'react' +import { screen, findByText } from '@testing-library/react' +import { describe, it, beforeEach, afterEach, expect, vi } from 'vitest' +import { when } from 'vitest-when' +import '@testing-library/jest-dom/vitest' +import { renderWithProviders } from '../../../__testing-utils__' +import { i18n } from '../../../i18n' +import { IncompatibleModuleTakeover } from '../IncompatibleModuleTakeover' +import { IncompatibleModuleODDModalBody } from '../IncompatibleModuleODDModalBody' +import { IncompatibleModuleDesktopModalBody } from '../IncompatibleModuleDesktopModalBody' +import { useIncompatibleModulesAttached } from '../hooks' +import type { AttachedModule } from '@opentrons/api-client' +import { + PortalRoot, + TopPortalRoot, + MODAL_PORTAL_ID, + TOP_PORTAL_ID, +} from '../../../App/portal' +import * as Fixtures from '../__fixtures__' + +vi.mock('../hooks') +vi.mock('../IncompatibleModuleODDModalBody') +vi.mock('../IncompatibleModuleDesktopModalBody') + +const getRenderer = (incompatibleModules: AttachedModule[]) => { + when(useIncompatibleModulesAttached) + .calledWith(expect.anything()) + .thenReturn(incompatibleModules) + vi.mocked(IncompatibleModuleODDModalBody).mockReturnValue( +
    TEST ELEMENT ODD
    + ) + vi.mocked(IncompatibleModuleDesktopModalBody).mockReturnValue( +
    TEST ELEMENT DESKTOP
    + ) + return (props: React.ComponentProps) => { + const [rendered] = renderWithProviders( + <> + + + + , + { + i18nInstance: i18n, + } + ) + rendered.rerender( + <> + + + + + ) + return rendered + } +} + +describe('IncompatibleModuleTakeover', () => { + let props: React.ComponentProps + beforeEach(() => { + props = { isOnDevice: true } + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + ;['desktop', 'odd'].forEach(target => { + it(`should render nothing on ${target} when no incompatible modules are attached`, () => { + getRenderer([])({ ...props, isOnDevice: target === 'odd' }) + expect(screen.findByTestId(TOP_PORTAL_ID)).resolves.toBeEmptyDOMElement() + expect( + screen.findByTestId(MODAL_PORTAL_ID) + ).resolves.toBeEmptyDOMElement() + expect(screen.queryByText(/TEST ELEMENT/)).toBeNull() + }) + }) + + it('should render the modal body on odd when incompatible modules are attached', async () => { + getRenderer(Fixtures.oneIncompatibleModule as any)({ + ...props, + isOnDevice: true, + }) + const container = await screen.findByTestId(TOP_PORTAL_ID) + await findByText(container, 'TEST ELEMENT ODD') + }) + + it('should render the modal body on desktop when incompatible modules are attached', async () => { + getRenderer(Fixtures.oneIncompatibleModule as any)({ + ...props, + isOnDevice: false, + }) + const container = await screen.findByTestId(MODAL_PORTAL_ID) + await findByText(container, 'TEST ELEMENT DESKTOP') + }) +}) diff --git a/app/src/organisms/IncompatibleModule/hooks/__fixtures__/incompatibleModuleFixtures.ts b/app/src/organisms/IncompatibleModule/hooks/__fixtures__/incompatibleModuleFixtures.ts new file mode 100644 index 00000000000..fbe7141b4ce --- /dev/null +++ b/app/src/organisms/IncompatibleModule/hooks/__fixtures__/incompatibleModuleFixtures.ts @@ -0,0 +1,386 @@ +export const mockModulesAllNotImplementedResponse = [ + { + id: '3feb840a3fa2dac2409b977f1e330f54f50e6231', + serialNumber: 'dummySerialTC', + firmwareVersion: 'dummyVersionTC', + hardwareRevision: 'dummyModelTC', + hasAvailableUpdate: false, + moduleType: 'thermocyclerModuleType', + moduleModel: 'thermocyclerModuleV1', + data: { + status: 'holding at target', + currentTemperature: 3.0, + targetTemperature: 3.0, + lidStatus: 'open', + lidTemperature: 4.0, + lidTargetTemperature: 4.0, + holdTime: 121.0, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, + { + id: '8bcc37fdfcb4c2b5ab69963c589ceb1f9b1d1c4f', + serialNumber: 'dummySerialHS', + firmwareVersion: 'dummyVersionHS', + hardwareRevision: 'dummyModelHS', + hasAvailableUpdate: false, + moduleType: 'heaterShakerModuleType', + moduleModel: 'heaterShakerModuleV1', + data: { + status: 'idle', + labwareLatchStatus: 'idle_unknown', + speedStatus: 'idle', + currentSpeed: 0, + temperatureStatus: 'idle', + currentTemperature: 23.0, + targetSpeed: null, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, + { + id: '5fe40b412e39c6c079125b5dd4820ad8044e0962', + serialNumber: 'dummySerialTD', + firmwareVersion: 'dummyVersionTD', + hardwareRevision: 'temp_deck_v1.1', + hasAvailableUpdate: false, + moduleType: 'temperatureModuleType', + moduleModel: 'temperatureModuleV1', + data: { + status: 'holding at target', + currentTemperature: 3.0, + targetTemperature: 3.0, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, + { + id: '67a5b5118a952417b4aa47a62a96deccb13bed32', + serialNumber: 'dummySerialMD', + firmwareVersion: 'dummyVersionMD', + hardwareRevision: 'mag_deck_v1.1', + hasAvailableUpdate: false, + moduleType: 'magneticModuleType', + moduleModel: 'magneticModuleV1', + data: { + status: 'engaged', + engaged: true, + height: 4.0, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, +] + +export const mockModulesAllCompatibleResponse = [ + { + id: '3feb840a3fa2dac2409b977f1e330f54f50e6231', + serialNumber: 'dummySerialTC', + firmwareVersion: 'dummyVersionTC', + hardwareRevision: 'dummyModelTC', + hasAvailableUpdate: false, + moduleType: 'thermocyclerModuleType', + moduleModel: 'thermocyclerModuleV1', + compatibleWithRobot: true, + data: { + status: 'holding at target', + currentTemperature: 3.0, + targetTemperature: 3.0, + lidStatus: 'open', + lidTemperature: 4.0, + lidTargetTemperature: 4.0, + holdTime: 121.0, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, + { + id: '8bcc37fdfcb4c2b5ab69963c589ceb1f9b1d1c4f', + serialNumber: 'dummySerialHS', + firmwareVersion: 'dummyVersionHS', + hardwareRevision: 'dummyModelHS', + hasAvailableUpdate: false, + moduleType: 'heaterShakerModuleType', + moduleModel: 'heaterShakerModuleV1', + compatibleWithRobot: true, + data: { + status: 'idle', + labwareLatchStatus: 'idle_unknown', + speedStatus: 'idle', + currentSpeed: 0, + temperatureStatus: 'idle', + currentTemperature: 23.0, + targetSpeed: null, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, + { + id: '5fe40b412e39c6c079125b5dd4820ad8044e0962', + serialNumber: 'dummySerialTD', + firmwareVersion: 'dummyVersionTD', + hardwareRevision: 'temp_deck_v1.1', + hasAvailableUpdate: false, + moduleType: 'temperatureModuleType', + moduleModel: 'temperatureModuleV1', + compatibleWithRobot: true, + data: { + status: 'holding at target', + currentTemperature: 3.0, + targetTemperature: 3.0, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, + { + id: '67a5b5118a952417b4aa47a62a96deccb13bed32', + serialNumber: 'dummySerialMD', + firmwareVersion: 'dummyVersionMD', + hardwareRevision: 'mag_deck_v1.1', + hasAvailableUpdate: false, + moduleType: 'magneticModuleType', + moduleModel: 'magneticModuleV1', + compatibleWithRobot: true, + data: { + status: 'engaged', + engaged: true, + height: 4.0, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, +] + +export const mockModulesWithOneIncompatibleResponse = [ + { + id: '3feb840a3fa2dac2409b977f1e330f54f50e6231', + serialNumber: 'dummySerialTC', + firmwareVersion: 'dummyVersionTC', + hardwareRevision: 'dummyModelTC', + hasAvailableUpdate: false, + moduleType: 'thermocyclerModuleType', + moduleModel: 'thermocyclerModuleV1', + compatibleWithRobot: false, + data: { + status: 'holding at target', + currentTemperature: 3.0, + targetTemperature: 3.0, + lidStatus: 'open', + lidTemperature: 4.0, + lidTargetTemperature: 4.0, + holdTime: 121.0, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, + { + id: '8bcc37fdfcb4c2b5ab69963c589ceb1f9b1d1c4f', + serialNumber: 'dummySerialHS', + firmwareVersion: 'dummyVersionHS', + hardwareRevision: 'dummyModelHS', + hasAvailableUpdate: false, + moduleType: 'heaterShakerModuleType', + moduleModel: 'heaterShakerModuleV1', + compatibleWithRobot: true, + data: { + status: 'idle', + labwareLatchStatus: 'idle_unknown', + speedStatus: 'idle', + currentSpeed: 0, + temperatureStatus: 'idle', + currentTemperature: 23.0, + targetSpeed: null, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, + { + id: '5fe40b412e39c6c079125b5dd4820ad8044e0962', + serialNumber: 'dummySerialTD', + firmwareVersion: 'dummyVersionTD', + hardwareRevision: 'temp_deck_v1.1', + hasAvailableUpdate: false, + moduleType: 'temperatureModuleType', + moduleModel: 'temperatureModuleV1', + compatibleWithRobot: true, + data: { + status: 'holding at target', + currentTemperature: 3.0, + targetTemperature: 3.0, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, + { + id: '67a5b5118a952417b4aa47a62a96deccb13bed32', + serialNumber: 'dummySerialMD', + firmwareVersion: 'dummyVersionMD', + hardwareRevision: 'mag_deck_v1.1', + hasAvailableUpdate: false, + moduleType: 'magneticModuleType', + moduleModel: 'magneticModuleV1', + compatibleWithRobot: true, + data: { + status: 'engaged', + engaged: true, + height: 4.0, + }, + usbPort: { + port: 0, + path: '', + hub: false, + portGroup: 'unknown', + }, + }, +] + +export const v2MockModulesResponse = [ + { + name: 'thermocycler', + displayName: 'thermocycler', + moduleModel: 'thermocyclerModuleV1', + port: '/dev/ot_module_sim_thermocycler0', + usbPort: { + port: 0, + hub: false, + portGroup: 'unknown', + path: '', + }, + serial: 'dummySerialTC', + model: 'dummyModelTC', + revision: 'dummyModelTC', + fwVersion: 'dummyVersionTC', + hasAvailableUpdate: false, + status: 'holding at target', + data: { + lid: 'open', + lidTarget: 4.0, + lidTemp: 4.0, + currentTemp: 3.0, + targetTemp: 3.0, + holdTime: 121.0, + rampRate: null, + currentCycleIndex: null, + totalCycleCount: null, + currentStepIndex: null, + totalStepCount: null, + }, + }, + { + name: 'heatershaker', + displayName: 'heatershaker', + moduleModel: 'heaterShakerModuleV1', + port: '/dev/ot_module_sim_heatershaker1', + usbPort: { + hub: false, + portGroup: 'unknown', + path: '', + port: 0, + }, + serial: 'dummySerialHS', + model: 'dummyModelHS', + revision: 'dummyModelHS', + fwVersion: 'dummyVersionHS', + hasAvailableUpdate: false, + status: 'idle', + data: { + labwareLatchStatus: 'idle_unknown', + speedStatus: 'idle', + temperatureStatus: 'idle', + currentSpeed: 0, + currentTemp: 23.0, + targetSpeed: null, + targetTemp: null, + errorDetails: null, + }, + }, + { + name: 'tempdeck', + displayName: 'tempdeck', + moduleModel: 'temperatureModuleV1', + port: '/dev/ot_module_sim_tempdeck2', + usbPort: { + hub: false, + portGroup: 'unknown', + path: '', + port: 0, + }, + serial: 'dummySerialTD', + model: 'temp_deck_v1.1', + revision: 'temp_deck_v1.1', + fwVersion: 'dummyVersionTD', + hasAvailableUpdate: false, + status: 'holding at target', + data: { + currentTemp: 3.0, + targetTemp: 3.0, + }, + }, + { + name: 'magdeck', + displayName: 'magdeck', + moduleModel: 'magneticModuleV1', + port: '/dev/ot_module_sim_magdeck3', + usbPort: { + port: 0, + hub: false, + portGroup: 'unknown', + path: '', + }, + serial: 'dummySerialMD', + model: 'mag_deck_v1.1', + revision: 'mag_deck_v1.1', + fwVersion: 'dummyVersionMD', + hasAvailableUpdate: false, + status: 'engaged', + data: { + engaged: true, + height: 4.0, + }, + }, +] diff --git a/app/src/organisms/IncompatibleModule/hooks/__fixtures__/index.ts b/app/src/organisms/IncompatibleModule/hooks/__fixtures__/index.ts new file mode 100644 index 00000000000..fc83332dba7 --- /dev/null +++ b/app/src/organisms/IncompatibleModule/hooks/__fixtures__/index.ts @@ -0,0 +1 @@ +export * from './incompatibleModuleFixtures' diff --git a/app/src/organisms/IncompatibleModule/hooks/__tests__/useIncompatibleModulesAttached.test.tsx b/app/src/organisms/IncompatibleModule/hooks/__tests__/useIncompatibleModulesAttached.test.tsx new file mode 100644 index 00000000000..970805c95c6 --- /dev/null +++ b/app/src/organisms/IncompatibleModule/hooks/__tests__/useIncompatibleModulesAttached.test.tsx @@ -0,0 +1,72 @@ +import * as React from 'react' +import { QueryClient, QueryClientProvider } from 'react-query' + +import { vi, it, expect, describe, beforeEach } from 'vitest' +import { renderHook } from '@testing-library/react' +import { useModulesQuery } from '@opentrons/react-api-client' +import { useIncompatibleModulesAttached } from '..' + +import * as Fixtures from '../__fixtures__' + +import type { Modules } from '@opentrons/api-client' +import type { UseQueryResult } from 'react-query' +vi.mock('@opentrons/react-api-client') + +describe('useIncompatibleModulesAttached', () => { + let wrapper: React.FunctionComponent<{ children: React.ReactNode }> + beforeEach(() => { + const queryClient = new QueryClient() + const clientProvider: React.FunctionComponent<{ + children: React.ReactNode + }> = ({ children }) => ( + {children} + ) + wrapper = clientProvider + }) + it('treats older endpoint responses as if the module were compatible', () => { + vi.mocked(useModulesQuery).mockReturnValue(({ + data: { + data: Fixtures.v2MockModulesResponse, + meta: {}, + }, + error: null, + } as any) as UseQueryResult) + const { result } = renderHook(useIncompatibleModulesAttached, { wrapper }) + expect(result.current).toHaveLength(0) + }) + it('pulls incompatible modules out of endpoint responses', () => { + vi.mocked(useModulesQuery).mockReturnValue(({ + data: { + data: Fixtures.mockModulesWithOneIncompatibleResponse, + meta: {}, + }, + error: null, + } as any) as UseQueryResult) + const { result } = renderHook(useIncompatibleModulesAttached, { wrapper }) + expect(result.current).toHaveLength(1) + expect(result.current).toContain( + Fixtures.mockModulesWithOneIncompatibleResponse[0] + ) + }) + it('treats modules under new schema without compatibility as compatible', () => { + vi.mocked(useModulesQuery).mockReturnValue(({ + data: { + data: Fixtures.mockModulesAllNotImplementedResponse, + meta: {}, + }, + error: null, + } as any) as UseQueryResult) + const { result } = renderHook(useIncompatibleModulesAttached, { wrapper }) + expect(result.current).toHaveLength(0) + }) + it('passes all compatible modules', () => { + vi.mocked(useModulesQuery).mockReturnValue(({ + data: { + data: Fixtures.mockModulesAllCompatibleResponse, + meta: {}, + }, + } as any) as UseQueryResult) + const { result } = renderHook(useIncompatibleModulesAttached, { wrapper }) + expect(result.current).toHaveLength(0) + }) +}) diff --git a/app/src/organisms/IncompatibleModule/hooks/index.ts b/app/src/organisms/IncompatibleModule/hooks/index.ts new file mode 100644 index 00000000000..9a5e1978747 --- /dev/null +++ b/app/src/organisms/IncompatibleModule/hooks/index.ts @@ -0,0 +1 @@ +export * from './useIncompatibleModulesAttached' diff --git a/app/src/organisms/IncompatibleModule/hooks/useIncompatibleModulesAttached.ts b/app/src/organisms/IncompatibleModule/hooks/useIncompatibleModulesAttached.ts new file mode 100644 index 00000000000..f57ba37d69c --- /dev/null +++ b/app/src/organisms/IncompatibleModule/hooks/useIncompatibleModulesAttached.ts @@ -0,0 +1,17 @@ +import { useModulesQuery } from '@opentrons/react-api-client' +import type { UseQueryOptions } from 'react-query' +import type { AttachedModule, Modules, HostConfig } from '@opentrons/api-client' + +export function useIncompatibleModulesAttached( + options: UseQueryOptions = {}, + hostOverride?: HostConfig | null +): AttachedModule[] { + const { data, error } = useModulesQuery({ + ...options, + }) + return error == null + ? data?.data.filter( + attachedModule => attachedModule?.compatibleWithRobot === false + ) || [] + : [] +} diff --git a/app/src/organisms/IncompatibleModule/index.tsx b/app/src/organisms/IncompatibleModule/index.tsx new file mode 100644 index 00000000000..e9866b2689c --- /dev/null +++ b/app/src/organisms/IncompatibleModule/index.tsx @@ -0,0 +1 @@ +export * from './IncompatibleModuleTakeover' diff --git a/robot-server/robot_server/modules/module_data_mapper.py b/robot-server/robot_server/modules/module_data_mapper.py index 4a501f2b32d..dafcf4e3fce 100644 --- a/robot-server/robot_server/modules/module_data_mapper.py +++ b/robot-server/robot_server/modules/module_data_mapper.py @@ -1,5 +1,8 @@ """Module identification and response data mapping.""" from typing import Type, cast, Optional +from fastapi import Depends + +from opentrons_shared_data.module import load_definition from opentrons.hardware_control.modules import ( LiveData, @@ -16,7 +19,7 @@ ) from opentrons.drivers.rpi_drivers.types import USBPort as HardwareUSBPort -from opentrons.protocol_engine import ModuleModel +from opentrons.protocol_engine import ModuleModel, DeckType from .module_identifier import ModuleIdentity from .module_models import ( @@ -34,10 +37,15 @@ UsbPort, ) +from robot_server.hardware import get_deck_type + class ModuleDataMapper: """Map hardware control modules to module response.""" + def __init__(self, deck_type: DeckType = Depends(get_deck_type)) -> None: + self.deck_type = deck_type + def map_data( self, model: str, @@ -53,6 +61,7 @@ def map_data( module_cls: Type[AttachedModule] module_data: AttachedModuleData + module_definition = load_definition(model_or_loadname=model, version="3") # rely on Pydantic to check/coerce data fields from dicts at run time if module_type == ModuleType.MAGNETIC: @@ -131,6 +140,9 @@ def map_data( firmwareVersion=module_identity.firmware_version, hardwareRevision=module_identity.hardware_revision, hasAvailableUpdate=has_available_update, + compatibleWithRobot=( + not (self.deck_type.value in module_definition["incompatibleWithDecks"]) + ), usbPort=UsbPort( port=usb_port.port_number, portGroup=usb_port.port_group, diff --git a/robot-server/robot_server/modules/module_models.py b/robot-server/robot_server/modules/module_models.py index a82e941fbb6..f05e2ff0a99 100644 --- a/robot-server/robot_server/modules/module_models.py +++ b/robot-server/robot_server/modules/module_models.py @@ -90,6 +90,9 @@ class _GenericModule(GenericModel, Generic[ModuleT, ModuleModelT, ModuleDataT]): moduleOffset: Optional[ModuleCalibrationData] = Field( None, description="The calibrated module offset." ) + compatibleWithRobot: bool = Field( + ..., description="Whether the detected module is compatible with this robot." + ) data: ModuleDataT usbPort: UsbPort diff --git a/robot-server/tests/integration/test_modules.tavern.yaml b/robot-server/tests/integration/test_modules.tavern.yaml index 815b736acf7..d08eabe1234 100644 --- a/robot-server/tests/integration/test_modules.tavern.yaml +++ b/robot-server/tests/integration/test_modules.tavern.yaml @@ -1,5 +1,5 @@ --- -test_name: Get modules +test_name: Get modules OT2 marks: - usefixtures: - ot2_server_base_url @@ -113,6 +113,7 @@ stages: firmwareVersion: !anystr hardwareRevision: !anystr hasAvailableUpdate: !anybool + compatibleWithRobot: true moduleType: thermocyclerModuleType moduleModel: thermocyclerModuleV1 usbPort: @@ -134,6 +135,7 @@ stages: firmwareVersion: !anystr hardwareRevision: !anystr hasAvailableUpdate: !anybool + compatibleWithRobot: true moduleType: heaterShakerModuleType moduleModel: heaterShakerModuleV1 usbPort: @@ -153,6 +155,7 @@ stages: firmwareVersion: !anystr hardwareRevision: !anystr hasAvailableUpdate: !anybool + compatibleWithRobot: true moduleType: temperatureModuleType moduleModel: temperatureModuleV1 usbPort: @@ -169,6 +172,7 @@ stages: firmwareVersion: !anystr hardwareRevision: !anystr hasAvailableUpdate: !anybool + compatibleWithRobot: true moduleType: magneticModuleType moduleModel: magneticModuleV2 usbPort: @@ -185,6 +189,7 @@ stages: firmwareVersion: !anystr hardwareRevision: !anystr hasAvailableUpdate: !anybool + compatibleWithRobot: true moduleType: magneticModuleType moduleModel: magneticModuleV1 usbPort: @@ -196,3 +201,95 @@ stages: status: !anystr height: !anyfloat engaged: !anybool + +--- +test_name: Get modules on Flex +marks: + - usefixtures: + - ot3_server_base_url +stages: + - name: Get all the modules + request: + url: '{ot3_server_base_url}/modules' + method: GET + response: + status_code: 200 + json: + meta: !anydict + data: + - id: !anystr + serialNumber: !anystr + firmwareVersion: !anystr + hardwareRevision: !anystr + hasAvailableUpdate: !anybool + compatibleWithRobot: true + moduleType: thermocyclerModuleType + moduleModel: thermocyclerModuleV2 + usbPort: + port: !anyint + hub: !anybool + portGroup: !anystr + path: !anystr + data: + status: !anystr + lidStatus: !anystr + lidTemperatureStatus: !anystr + lidTargetTemperature: !anyfloat + lidTemperature: !anyfloat + currentTemperature: !anyfloat + targetTemperature: !anyfloat + holdTime: !anyfloat + - id: !anystr + serialNumber: !anystr + firmwareVersion: !anystr + hardwareRevision: !anystr + hasAvailableUpdate: !anybool + compatibleWithRobot: true + moduleType: heaterShakerModuleType + moduleModel: heaterShakerModuleV1 + usbPort: + port: !anyint + hub: !anybool + portGroup: !anystr + path: !anystr + data: + status: !anystr + labwareLatchStatus: !anystr + speedStatus: !anystr + temperatureStatus: !anystr + currentSpeed: !anyint + currentTemperature: !anyfloat + - id: !anystr + serialNumber: !anystr + firmwareVersion: !anystr + hardwareRevision: !anystr + hasAvailableUpdate: !anybool + compatibleWithRobot: true + moduleType: temperatureModuleType + moduleModel: temperatureModuleV2 + usbPort: + port: !anyint + hub: !anybool + portGroup: !anystr + path: !anystr + data: + status: !anystr + currentTemperature: !anyfloat + targetTemperature: !anyfloat + - id: !anystr + serialNumber: !anystr + firmwareVersion: !anystr + hardwareRevision: !anystr + hasAvailableUpdate: !anybool + compatibleWithRobot: true + moduleType: temperatureModuleType + moduleModel: temperatureModuleV2 + usbPort: + port: !anyint + hub: !anybool + portGroup: !anystr + path: !anystr + data: + status: !anystr + currentTemperature: !anyfloat + targetTemperature: !anyfloat diff --git a/robot-server/tests/modules/test_module_data_mapper.py b/robot-server/tests/modules/test_module_data_mapper.py index 7eb50854428..62fa54e9a49 100644 --- a/robot-server/tests/modules/test_module_data_mapper.py +++ b/robot-server/tests/modules/test_module_data_mapper.py @@ -1,7 +1,7 @@ """Tests for robot_server.modules.module_data_mapper.""" import pytest -from opentrons.protocol_engine import ModuleModel +from opentrons.protocol_engine import ModuleModel, DeckType from opentrons.protocol_engine.types import Vec3f from opentrons.drivers.rpi_drivers.types import USBPort as HardwareUSBPort, PortGroup from opentrons.hardware_control.modules import ( @@ -31,50 +31,88 @@ @pytest.mark.parametrize( - ("input_model", "input_data", "expected_output_data"), + ( + "input_model", + "deck_type", + "input_data", + "expected_output_data", + "expected_compatible", + ), [ ( "magneticModuleV1", + DeckType("ot2_standard"), {"status": "disengaged", "data": {"engaged": False, "height": 0.0}}, MagneticModuleData( status=MagneticStatus.DISENGAGED, engaged=False, height=-2.5, ), + True, ), ( "magneticModuleV1", + DeckType("ot3_standard"), + {"status": "disengaged", "data": {"engaged": False, "height": 0.0}}, + MagneticModuleData( + status=MagneticStatus.DISENGAGED, + engaged=False, + height=-2.5, + ), + False, + ), + ( + "magneticModuleV1", + DeckType("ot2_standard"), {"status": "engaged", "data": {"engaged": True, "height": 42}}, MagneticModuleData( status=MagneticStatus.ENGAGED, engaged=True, height=18.5, ), + True, ), ( "magneticModuleV2", + DeckType("ot2_standard"), {"status": "disengaged", "data": {"engaged": False, "height": 0.0}}, MagneticModuleData( status=MagneticStatus.DISENGAGED, engaged=False, height=-2.5, ), + True, ), ( "magneticModuleV2", + DeckType("ot3_standard"), + {"status": "disengaged", "data": {"engaged": False, "height": 0.0}}, + MagneticModuleData( + status=MagneticStatus.DISENGAGED, + engaged=False, + height=-2.5, + ), + False, + ), + ( + "magneticModuleV2", + DeckType("ot2_standard"), {"status": "engaged", "data": {"engaged": True, "height": 42}}, MagneticModuleData( status=MagneticStatus.ENGAGED, engaged=True, height=39.5, ), + True, ), ], ) def test_maps_magnetic_module_data( input_model: str, + deck_type: DeckType, input_data: LiveData, expected_output_data: MagneticModuleData, + expected_compatible: bool, ) -> None: """It should map hardware data to a magnetic module.""" module_identity = ModuleIdentity( @@ -93,7 +131,7 @@ def test_maps_magnetic_module_data( device_path="/dev/null", ) - subject = ModuleDataMapper() + subject = ModuleDataMapper(deck_type=deck_type) result = subject.map_data( model=input_model, module_identity=module_identity, @@ -113,6 +151,7 @@ def test_maps_magnetic_module_data( hasAvailableUpdate=True, moduleType=ModuleType.MAGNETIC, moduleModel=ModuleModel(input_model), # type: ignore[arg-type] + compatibleWithRobot=expected_compatible, usbPort=UsbPort( port=101, portGroup=PortGroup.UNKNOWN, @@ -126,8 +165,13 @@ def test_maps_magnetic_module_data( @pytest.mark.parametrize( - "input_model", - ["temperatureModuleV1", "temperatureModuleV2"], + "input_model,deck_type,expected_compatible", + [ + ("temperatureModuleV1", DeckType("ot2_standard"), True), + ("temperatureModuleV1", DeckType("ot3_standard"), False), + ("temperatureModuleV2", DeckType("ot2_standard"), True), + ("temperatureModuleV2", DeckType("ot3_standard"), True), + ], ) @pytest.mark.parametrize( "input_data", @@ -139,7 +183,12 @@ def test_maps_magnetic_module_data( }, ], ) -def test_maps_temperature_module_data(input_model: str, input_data: LiveData) -> None: +def test_maps_temperature_module_data( + input_model: str, + deck_type: DeckType, + expected_compatible: bool, + input_data: LiveData, +) -> None: """It should map hardware data to a magnetic module.""" module_identity = ModuleIdentity( module_id="module-id", @@ -157,7 +206,7 @@ def test_maps_temperature_module_data(input_model: str, input_data: LiveData) -> device_path="/dev/null", ) - subject = ModuleDataMapper() + subject = ModuleDataMapper(deck_type=deck_type) result = subject.map_data( model=input_model, module_identity=module_identity, @@ -176,6 +225,7 @@ def test_maps_temperature_module_data(input_model: str, input_data: LiveData) -> hardwareRevision="4.5.6", hasAvailableUpdate=True, moduleType=ModuleType.TEMPERATURE, + compatibleWithRobot=expected_compatible, moduleModel=ModuleModel(input_model), # type: ignore[arg-type] usbPort=UsbPort( port=101, @@ -194,8 +244,13 @@ def test_maps_temperature_module_data(input_model: str, input_data: LiveData) -> @pytest.mark.parametrize( - "input_model", - ["thermocyclerModuleV1"], + "input_model,deck_type,expected_compatible", + [ + ("thermocyclerModuleV1", DeckType("ot2_standard"), True), + ("thermocyclerModuleV1", DeckType("ot3_standard"), False), + ("thermocyclerModuleV2", DeckType("ot2_standard"), True), + ("thermocyclerModuleV2", DeckType("ot3_standard"), True), + ], ) @pytest.mark.parametrize( "input_data", @@ -236,7 +291,12 @@ def test_maps_temperature_module_data(input_model: str, input_data: LiveData) -> }, ], ) -def test_maps_thermocycler_module_data(input_model: str, input_data: LiveData) -> None: +def test_maps_thermocycler_module_data( + input_model: str, + deck_type: DeckType, + expected_compatible: bool, + input_data: LiveData, +) -> None: """It should map hardware data to a magnetic module.""" module_identity = ModuleIdentity( module_id="module-id", @@ -254,7 +314,7 @@ def test_maps_thermocycler_module_data(input_model: str, input_data: LiveData) - device_path="/dev/null", ) - subject = ModuleDataMapper() + subject = ModuleDataMapper(deck_type=deck_type) result = subject.map_data( model=input_model, module_identity=module_identity, @@ -273,6 +333,7 @@ def test_maps_thermocycler_module_data(input_model: str, input_data: LiveData) - hardwareRevision="4.5.6", hasAvailableUpdate=True, moduleType=ModuleType.THERMOCYCLER, + compatibleWithRobot=expected_compatible, moduleModel=ModuleModel(input_model), # type: ignore[arg-type] usbPort=UsbPort( port=101, @@ -301,8 +362,11 @@ def test_maps_thermocycler_module_data(input_model: str, input_data: LiveData) - @pytest.mark.parametrize( - "input_model", - ["heaterShakerModuleV1"], + "input_model,deck_type", + [ + ("heaterShakerModuleV1", DeckType("ot2_standard")), + ("heaterShakerModuleV1", DeckType("ot3_standard")), + ], ) @pytest.mark.parametrize( "input_data", @@ -335,7 +399,9 @@ def test_maps_thermocycler_module_data(input_model: str, input_data: LiveData) - }, ], ) -def test_maps_heater_shaker_module_data(input_model: str, input_data: LiveData) -> None: +def test_maps_heater_shaker_module_data( + input_model: str, deck_type: DeckType, input_data: LiveData +) -> None: """It should map hardware data to a magnetic module.""" module_identity = ModuleIdentity( module_id="module-id", @@ -353,7 +419,7 @@ def test_maps_heater_shaker_module_data(input_model: str, input_data: LiveData) device_path="/dev/null", ) - subject = ModuleDataMapper() + subject = ModuleDataMapper(deck_type=deck_type) result = subject.map_data( model=input_model, module_identity=module_identity, @@ -372,6 +438,7 @@ def test_maps_heater_shaker_module_data(input_model: str, input_data: LiveData) hardwareRevision="4.5.6", hasAvailableUpdate=True, moduleType=ModuleType.HEATER_SHAKER, + compatibleWithRobot=True, moduleModel=ModuleModel(input_model), # type: ignore[arg-type] usbPort=UsbPort( port=101, diff --git a/robot-server/tests/modules/test_router.py b/robot-server/tests/modules/test_router.py index 6aa92b3fee7..f2392672109 100644 --- a/robot-server/tests/modules/test_router.py +++ b/robot-server/tests/modules/test_router.py @@ -49,7 +49,9 @@ def module_identifier(decoy: Decoy) -> ModuleIdentifier: @pytest.fixture() -def module_data_mapper(decoy: Decoy) -> ModuleDataMapper: +def module_data_mapper( + decoy: Decoy, +) -> ModuleDataMapper: """Get a mock module data mapper.""" return decoy.mock(cls=ModuleDataMapper) @@ -87,6 +89,7 @@ async def test_get_modules_maps_data_and_id( hasAvailableUpdate=True, moduleType=ModuleType.MAGNETIC, moduleModel=ModuleModel.MAGNETIC_MODULE_V1, + compatibleWithRobot=True, usbPort=UsbPort( port=42, hub=False, diff --git a/shared-data/module/definitions/3/absorbanceReaderV1.json b/shared-data/module/definitions/3/absorbanceReaderV1.json index 7b3172c377d..bed2c21302e 100644 --- a/shared-data/module/definitions/3/absorbanceReaderV1.json +++ b/shared-data/module/definitions/3/absorbanceReaderV1.json @@ -69,6 +69,7 @@ } }, "compatibleWith": [], + "incompatibleWithDecks": ["ot2_standard"], "twoDimensionalRendering": { "name": "svg", "type": "element", diff --git a/shared-data/module/definitions/3/heaterShakerModuleV1.json b/shared-data/module/definitions/3/heaterShakerModuleV1.json index fac8fa81e8d..46385cf1bbf 100644 --- a/shared-data/module/definitions/3/heaterShakerModuleV1.json +++ b/shared-data/module/definitions/3/heaterShakerModuleV1.json @@ -170,6 +170,7 @@ } }, "compatibleWith": [], + "incompatibleWithDecks": [], "twoDimensionalRendering": { "name": "svg", "type": "element", diff --git a/shared-data/module/definitions/3/magneticBlockV1.json b/shared-data/module/definitions/3/magneticBlockV1.json index a1fd0a39248..0ce8ca3ffa6 100644 --- a/shared-data/module/definitions/3/magneticBlockV1.json +++ b/shared-data/module/definitions/3/magneticBlockV1.json @@ -50,6 +50,7 @@ "ot3_standard": {} }, "compatibleWith": [], + "incompatibleWithDecks": [], "twoDimensionalRendering": { "name": "svg", "type": "element", diff --git a/shared-data/module/definitions/3/magneticModuleV1.json b/shared-data/module/definitions/3/magneticModuleV1.json index ab46b018ae5..eb31487876b 100644 --- a/shared-data/module/definitions/3/magneticModuleV1.json +++ b/shared-data/module/definitions/3/magneticModuleV1.json @@ -34,6 +34,7 @@ "quirks": [], "slotTransforms": {}, "compatibleWith": [], + "incompatibleWithDecks": ["ot3_standard"], "twoDimensionalRendering": { "name": "svg", "type": "element", diff --git a/shared-data/module/definitions/3/magneticModuleV2.json b/shared-data/module/definitions/3/magneticModuleV2.json index 5344627ce35..9bc35e5c2dd 100644 --- a/shared-data/module/definitions/3/magneticModuleV2.json +++ b/shared-data/module/definitions/3/magneticModuleV2.json @@ -87,6 +87,7 @@ } }, "compatibleWith": [], + "incompatibleWithDecks": ["ot3_standard"], "twoDimensionalRendering": { "name": "svg", "type": "element", diff --git a/shared-data/module/definitions/3/temperatureModuleV1.json b/shared-data/module/definitions/3/temperatureModuleV1.json index 9b7b703ba52..7f5d824e1b5 100644 --- a/shared-data/module/definitions/3/temperatureModuleV1.json +++ b/shared-data/module/definitions/3/temperatureModuleV1.json @@ -35,6 +35,7 @@ "quirks": [], "slotTransforms": {}, "compatibleWith": ["temperatureModuleV2"], + "incompatibleWithDecks": ["ot3_standard"], "twoDimensionalRendering": { "name": "svg", "type": "element", diff --git a/shared-data/module/definitions/3/temperatureModuleV2.json b/shared-data/module/definitions/3/temperatureModuleV2.json index 07bdfe03c47..ecd353eb0bd 100644 --- a/shared-data/module/definitions/3/temperatureModuleV2.json +++ b/shared-data/module/definitions/3/temperatureModuleV2.json @@ -168,6 +168,7 @@ } }, "compatibleWith": ["temperatureModuleV1"], + "incompatibleWithDecks": [], "twoDimensionalRendering": { "name": "svg", "type": "element", diff --git a/shared-data/module/definitions/3/thermocyclerModuleV1.json b/shared-data/module/definitions/3/thermocyclerModuleV1.json index 09dd6828c9b..a6933bb3db5 100644 --- a/shared-data/module/definitions/3/thermocyclerModuleV1.json +++ b/shared-data/module/definitions/3/thermocyclerModuleV1.json @@ -38,6 +38,7 @@ "quirks": [], "slotTransforms": {}, "compatibleWith": ["thermocyclerModuleV1"], + "incompatibleWithDecks": ["ot3_standard"], "twoDimensionalRendering": { "name": "svg", "type": "element", diff --git a/shared-data/module/definitions/3/thermocyclerModuleV2.json b/shared-data/module/definitions/3/thermocyclerModuleV2.json index b5d8b1fbd9e..f051d81078b 100644 --- a/shared-data/module/definitions/3/thermocyclerModuleV2.json +++ b/shared-data/module/definitions/3/thermocyclerModuleV2.json @@ -69,6 +69,7 @@ } }, "compatibleWith": [], + "incompatibleWithDecks": [], "twoDimensionalRendering": { "name": "svg", "type": "element", diff --git a/shared-data/module/schemas/3.json b/shared-data/module/schemas/3.json index fdafba4c8a9..c422645a67a 100644 --- a/shared-data/module/schemas/3.json +++ b/shared-data/module/schemas/3.json @@ -67,7 +67,8 @@ "quirks", "slotTransforms", "compatibleWith", - "twoDimensionalRendering" + "twoDimensionalRendering", + "incompatibleWithDecks" ], "additionalProperties": false, "properties": { @@ -198,6 +199,13 @@ "description": "A compatible module model (e.g. temperatureModuleV1)" } }, + "incompatibleWithDecks": { + "type": "array", + "items": { + "type": "string", + "description": "A list of robot decks (by their definition name) not compatible with this module." + } + }, "twoDimensionalRendering": { "type": "object", "description": "SVG rendering of the module represented as svgson", diff --git a/shared-data/python/opentrons_shared_data/module/dev_types.py b/shared-data/python/opentrons_shared_data/module/dev_types.py index c6dbbc4acad..827905d8a31 100644 --- a/shared-data/python/opentrons_shared_data/module/dev_types.py +++ b/shared-data/python/opentrons_shared_data/module/dev_types.py @@ -109,6 +109,7 @@ class GripperOffsets(TypedDict): "quirks": List[str], "slotTransforms": Dict[str, Dict[str, Dict[str, List[List[float]]]]], "compatibleWith": List[ModuleModel], + "incompatibleWithDecks": List[str], "twoDimensionalRendering": Dict[str, Any], "gripperOffsets": Dict[str, GripperOffsets], }, From f76fea1dcb54b340c328b94d1e4b6b12636466b3 Mon Sep 17 00:00:00 2001 From: Rhyann Clarke <146747548+rclarke0@users.noreply.github.com> Date: Tue, 21 May 2024 17:24:27 -0400 Subject: [PATCH 05/17] Changed write row commands to batch write cells (#15231) # Overview Converted write row commands to batch write cells for faster data recording # Test Plan # Changelog # Review requests # Risk assessment --- .../automation/google_sheets_tool.py | 73 ++++++++++++++----- .../data_collection/abr_google_drive.py | 34 +++++---- .../data_collection/abr_robot_error.py | 12 +-- .../data_collection/read_robot_logs.py | 12 +-- .../data_collection/single_run_log_reader.py | 16 +--- 5 files changed, 92 insertions(+), 55 deletions(-) diff --git a/abr-testing/abr_testing/automation/google_sheets_tool.py b/abr-testing/abr_testing/automation/google_sheets_tool.py index e4b8b6499e0..afac386bc40 100644 --- a/abr-testing/abr_testing/automation/google_sheets_tool.py +++ b/abr-testing/abr_testing/automation/google_sheets_tool.py @@ -6,7 +6,7 @@ import sys from datetime import datetime from oauth2client.service_account import ServiceAccountCredentials # type: ignore[import] -from typing import Dict, List, Any, Set, Tuple +from typing import Dict, List, Any, Set, Tuple, Optional """Google Sheets Tool. @@ -48,13 +48,14 @@ def open_worksheet(self, tab_number: int) -> Any: """Open individual worksheet within a googlesheet.""" return self.spread_sheet.get_worksheet(tab_number) - def create_worksheet(self, title: str) -> None: + def create_worksheet(self, title: str) -> Optional[str]: """Create a worksheet with tab name. Existing spreadsheet needed.""" try: - new_sheet = self.spread_sheet.add_worksheet(title, rows="2000", cols="26") + new_sheet = self.spread_sheet.add_worksheet(title, rows="2500", cols="40") return new_sheet.id except gspread.exceptions.APIError: print("Sheet already exists.") + return new_sheet.id def write_header(self, header: List) -> None: """Write Header to first row if not present.""" @@ -108,17 +109,55 @@ def batch_delete_rows(self, row_indices: List[int]) -> None: self.spread_sheet.batch_update(body=delete_body) def batch_update_cells( - self, sheet_title: str, data: List[List[str]], start_column: str, start_row: int + self, + data: List[List[Any]], + start_column: str, + start_row: int, + sheet_id: str, ) -> None: """Writes to multiple cells at once in a specific sheet.""" - sheet = self.spread_sheet.worksheet(sheet_title) - for idx, values in enumerate(data): - column = chr(ord(start_column) + idx) # Convert index to column letter - location = f"{column}{start_row}:{column}{start_row + len(values) - 1}" - cells_to_update = sheet.range(location) - for cell, value in zip(cells_to_update, values): - cell.value = value - sheet.update_cells(cells_to_update) + + def column_letter_to_index(column_letter: str) -> int: + """Convert a column letter (e.g., 'A') to a 1-based column index (e.g., 1).""" + index = 0 + for char in column_letter.upper(): + index = index * 26 + (ord(char) - ord("A") + 1) + return index + + requests = [] + user_entered_value: Dict[str, Any] = {} + start_column_index = column_letter_to_index(start_column) - 1 + + for col_offset, col_values in enumerate(data): + column_index = start_column_index + col_offset + # column_letter = index_to_column_letter(column_index) + for row_offset, value in enumerate(col_values): + row_index = start_row + row_offset + try: + float_value = float(value) + user_entered_value = {"numberValue": float_value} + except ValueError: + user_entered_value = {"stringValue": str(value)} + requests.append( + { + "updateCells": { + "range": { + "sheetId": sheet_id, + "startRowIndex": row_index - 1, + "endRowIndex": row_index, + "startColumnIndex": column_index, + "endColumnIndex": column_index + 1, + }, + "rows": [ + {"values": [{"userEnteredValue": user_entered_value}]} + ], + "fields": "userEnteredValue", + } + } + ) + + body = {"requests": requests} + self.spread_sheet.batch_update(body=body) def update_cell( self, sheet_title: str, row: int, column: int, single_data: Any @@ -135,13 +174,13 @@ def get_column(self, column_number: int) -> Set[str]: """Get all values in column.""" return self.worksheet.col_values(column_number) - def get_cell(self, cell: str) -> Any: + def get_cell(self, sheet_title: str, cell: str) -> Any: """Get cell value with location ex: 'A1'.""" - return self.worksheet.acell(cell).value + return self.spread_sheet.worksheet(sheet_title).acell(cell).value - def get_single_col_range(self, range: str) -> List: + def get_single_col_range(self, sheet_name: str, range: str) -> List: """Get cell values from one column range.""" - values_range = self.worksheet.range(range) + values_range = self.spread_sheet.worksheet(sheet_name).range(range) return [cell.value for cell in values_range] def get_index_row(self) -> int: @@ -214,7 +253,7 @@ def create_line_chart( "overlayPosition": { "anchorCell": { "sheetId": sheet_id, - "rowIndex": 1, + "rowIndex": 15, "columnIndex": col_position, } } diff --git a/abr-testing/abr_testing/data_collection/abr_google_drive.py b/abr-testing/abr_testing/data_collection/abr_google_drive.py index a2a180c2bd5..b723ffe866d 100644 --- a/abr-testing/abr_testing/data_collection/abr_google_drive.py +++ b/abr-testing/abr_testing/data_collection/abr_google_drive.py @@ -32,10 +32,10 @@ def create_data_dictionary( runs_to_save: Union[Set[str], str], storage_directory: str, issue_url: str, -) -> Tuple[Dict[str, Dict[str, Any]], List[str], Dict[str, Dict[str, Any]], List[str]]: +) -> Tuple[List[List[Any]], List[str], List[List[Any]], List[str]]: """Pull data from run files and format into a dictionary.""" - runs_and_robots: Dict[Any, Dict[str, Any]] = {} - runs_and_lpc: Dict[Any, Dict[str, Any]] = {} + runs_and_robots: List[Any] = [] + runs_and_lpc: List[Dict[str, Any]] = [] for filename in os.listdir(storage_directory): file_path = os.path.join(storage_directory, filename) if file_path.endswith(".json"): @@ -119,14 +119,17 @@ def create_data_dictionary( **tc_dict, } headers: List[str] = list(row_2.keys()) - runs_and_robots[run_id] = row_2 + # runs_and_robots[run_id] = row_2 + runs_and_robots.append(list(row_2.values())) # LPC Data Recording runs_and_lpc, headers_lpc = read_robot_logs.lpc_data( file_results, row_for_lpc, runs_and_lpc ) else: continue - return runs_and_robots, headers, runs_and_lpc, headers_lpc + transposed_runs_and_robots = list(map(list, zip(*runs_and_robots))) + transposed_runs_and_lpc = list(map(list, zip(*runs_and_lpc))) + return transposed_runs_and_robots, headers, transposed_runs_and_lpc, headers_lpc if __name__ == "__main__": @@ -183,14 +186,19 @@ def create_data_dictionary( run_ids_on_gd, run_ids_on_gs ) # Add missing runs to google sheet - runs_and_robots, headers, runs_and_lpc, headers_lpc = create_data_dictionary( - missing_runs_from_gs, storage_directory, "" - ) - read_robot_logs.write_to_local_and_google_sheet( - runs_and_robots, storage_directory, google_sheet_name, google_sheet, headers - ) + ( + transposed_runs_and_robots, + headers, + transposed_runs_and_lpc, + headers_lpc, + ) = create_data_dictionary(missing_runs_from_gs, storage_directory, "") + + start_row = google_sheet.get_index_row() + 1 + google_sheet.batch_update_cells(transposed_runs_and_robots, "A", start_row, "0") + # Add LPC to google sheet google_sheet_lpc = google_sheets_tool.google_sheet(credentials_path, "ABR-LPC", 0) - read_robot_logs.write_to_local_and_google_sheet( - runs_and_lpc, storage_directory, "ABR-LPC", google_sheet_lpc, headers_lpc + start_row_lpc = google_sheet_lpc.get_index_row() + 1 + google_sheet_lpc.batch_update_cells( + transposed_runs_and_lpc, "A", start_row_lpc, "0" ) diff --git a/abr-testing/abr_testing/data_collection/abr_robot_error.py b/abr-testing/abr_testing/data_collection/abr_robot_error.py index 2d0f35e2de5..23bc4f8e9ba 100644 --- a/abr-testing/abr_testing/data_collection/abr_robot_error.py +++ b/abr-testing/abr_testing/data_collection/abr_robot_error.py @@ -47,6 +47,7 @@ def get_error_info_from_robot( ) = read_robot_logs.get_error_info(results) # JIRA Ticket Fields failure_level = "Level " + str(error_level) + " Failure" + components = [failure_level, "Flex-RABR"] affects_version = results["API_Version"] parent = results.get("robot_name", "") @@ -201,12 +202,11 @@ def get_error_info_from_robot( runs_and_lpc, headers_lpc, ) = abr_google_drive.create_data_dictionary(run_id, error_folder_path, issue_url) - read_robot_logs.write_to_local_and_google_sheet( - runs_and_robots, storage_directory, google_sheet_name, google_sheet, headers - ) + + start_row = google_sheet.get_index_row() + 1 + google_sheet.batch_update_cells(runs_and_robots, "A", start_row, "0") print("Wrote run to ABR-run-data") # Add LPC to google sheet google_sheet_lpc = google_sheets_tool.google_sheet(credentials_path, "ABR-LPC", 0) - read_robot_logs.write_to_local_and_google_sheet( - runs_and_lpc, storage_directory, "ABR-LPC", google_sheet_lpc, headers_lpc - ) + start_row_lpc = google_sheet_lpc.get_index_row() + 1 + google_sheet_lpc.batch_update_cells(runs_and_lpc, "A", start_row_lpc, "0") diff --git a/abr-testing/abr_testing/data_collection/read_robot_logs.py b/abr-testing/abr_testing/data_collection/read_robot_logs.py index b1d5dcd9ead..37577f1a7ab 100644 --- a/abr-testing/abr_testing/data_collection/read_robot_logs.py +++ b/abr-testing/abr_testing/data_collection/read_robot_logs.py @@ -18,11 +18,10 @@ def lpc_data( file_results: Dict[str, Any], protocol_info: Dict[str, Any], - runs_and_lpc: Dict[str, Any], -) -> Tuple[Dict[str, Dict[str, Any]], List[str]]: + runs_and_lpc: List[Dict[str, Any]], +) -> Tuple[List[Dict[str, Any]], List[str]]: """Get labware offsets from one run log.""" offsets = file_results.get("labwareOffsets", "") - n = 0 # TODO: per UNIQUE slot AND LABWARE TYPE only keep the most recent LPC recording if len(offsets) > 0: unique_offsets: Dict[Any, Any] = {} @@ -55,9 +54,7 @@ def lpc_data( "Z": z_offset, } for item in unique_offsets: - run_id = protocol_info["Run_ID"] + "_" + str(n) - runs_and_lpc[run_id] = unique_offsets[item] - n += 1 + runs_and_lpc.append(unique_offsets[item].values()) headers_lpc = list(unique_offsets[(slot, labware_type)].keys()) return runs_and_lpc, headers_lpc @@ -298,6 +295,7 @@ def create_abr_data_sheet( def get_error_info(file_results: Dict[str, Any]) -> Tuple[int, str, str, str, str]: """Determines if errors exist in run log and documents them.""" error_levels = [] + error_level = "" # Read error levels file with open(ERROR_LEVELS_PATH, "r") as error_file: error_levels = list(csv.reader(error_file)) @@ -328,6 +326,8 @@ def get_error_info(file_results: Dict[str, Any]) -> Tuple[int, str, str, str, st code_error = error[1] if code_error == error_code: error_level = error[4] + if len(error_level) < 1: + error_level = str(4) return num_of_errors, error_type, error_code, error_instrument, error_level diff --git a/abr-testing/abr_testing/data_collection/single_run_log_reader.py b/abr-testing/abr_testing/data_collection/single_run_log_reader.py index b16ffd1df97..aa4aa5fe432 100644 --- a/abr-testing/abr_testing/data_collection/single_run_log_reader.py +++ b/abr-testing/abr_testing/data_collection/single_run_log_reader.py @@ -15,16 +15,8 @@ nargs=1, help="Folder path that holds individual run logs of interest.", ) - parser.add_argument( - "google_sheet_name", - metavar="GOOGLE_SHEET_NAME", - type=str, - nargs=1, - help="Google sheet name.", - ) args = parser.parse_args() run_log_file_path = args.run_log_file_path[0] - google_sheet_name = args.google_sheet_name[0] try: credentials_path = os.path.join(run_log_file_path, "credentials.json") @@ -41,7 +33,7 @@ ) = abr_google_drive.create_data_dictionary( run_ids_in_storage, run_log_file_path, "" ) - list_of_runs = list(runs_and_robots.keys()) + transposed_list = list(zip(*runs_and_robots)) # Adds Run to local csv sheet_location = os.path.join(run_log_file_path, "saved_data.csv") file_exists = os.path.exists(sheet_location) and os.path.getsize(sheet_location) > 0 @@ -49,8 +41,6 @@ writer = csv.writer(f) if not file_exists: writer.writerow(header) - for run in list_of_runs: + for run in transposed_list: # Add new row - row = runs_and_robots[run].values() - row_list = list(row) - writer.writerow(row_list) + writer.writerow(run) From 2ede48cdc169aad4ca532bf057e69589773e5749 Mon Sep 17 00:00:00 2001 From: Jethary Rader <66035149+jerader@users.noreply.github.com> Date: Wed, 22 May 2024 06:54:53 -0400 Subject: [PATCH 06/17] fix(protocol-designer): submit and don't show again fixes (#15230) closes RQA-2715 RQA-2719 --- protocol-designer/src/components/FilePage.tsx | 13 +++++++++---- protocol-designer/src/components/Hints/index.tsx | 10 ++++++---- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/protocol-designer/src/components/FilePage.tsx b/protocol-designer/src/components/FilePage.tsx index 4df3bdf583d..ac61114364e 100644 --- a/protocol-designer/src/components/FilePage.tsx +++ b/protocol-designer/src/components/FilePage.tsx @@ -83,7 +83,7 @@ export const FilePage = (): JSX.Element => { const saveFileMetadata = (nextFormValues: FileMetadataFields): void => { dispatch(actions.saveFileMetadata(nextFormValues)) } - + const [isManualDirty, setManualDirty] = React.useState(false) const { handleSubmit, watch, @@ -91,7 +91,6 @@ export const FilePage = (): JSX.Element => { setValue, formState: { isDirty }, } = useForm({ defaultValues: formValues }) - // to ensure that values from watch are up to date if the defaultValues // change React.useEffect(() => { @@ -155,6 +154,7 @@ export const FilePage = (): JSX.Element => { name="protocolName" value={protocolName} onChange={field.onChange} + onClick={() => setManualDirty(true)} /> )} /> @@ -172,6 +172,7 @@ export const FilePage = (): JSX.Element => { name="author" value={author} onChange={field.onChange} + onClick={() => setManualDirty(true)} /> )} /> @@ -190,6 +191,7 @@ export const FilePage = (): JSX.Element => { name="description" value={description} onChange={field.onChange} + onClick={() => setManualDirty(true)} /> )} /> @@ -198,9 +200,12 @@ export const FilePage = (): JSX.Element => { setManualDirty(false)} > - {isDirty ? t('application:update') : t('application:updated')} + {isManualDirty + ? t('application:update') + : t('application:updated')} diff --git a/protocol-designer/src/components/Hints/index.tsx b/protocol-designer/src/components/Hints/index.tsx index 6f5bafd2527..272096ea7e4 100644 --- a/protocol-designer/src/components/Hints/index.tsx +++ b/protocol-designer/src/components/Hints/index.tsx @@ -21,9 +21,13 @@ const HINT_IS_ALERT: HintKey[] = ['add_liquids_and_labware'] export const Hints = (): JSX.Element | null => { const { t } = useTranslation(['alert', 'nav', 'button']) - const [rememberDismissal, toggleRememberDismissal] = React.useState( + const [rememberDismissal, setRememberDismissal] = React.useState( false ) + + const toggleRememberDismissal = React.useCallback(() => { + setRememberDismissal(prevDismissal => !prevDismissal) + }, []) const hintKey = useSelector(selectors.getHint) const dispatch = useDispatch() const removeHint = (hintKey: HintKey): void => { @@ -159,9 +163,7 @@ export const Hints = (): JSX.Element | null => { { - toggleRememberDismissal(rememberDismissal) - }} + onChange={toggleRememberDismissal} value={rememberDismissal} /> Date: Wed, 22 May 2024 08:54:34 -0400 Subject: [PATCH 07/17] feat(app): "Cancel run" during Error Recovery (#15240) Closes EXEC-462 Adds the ability to cancel a run during Error Recovery. --- .../localization/en/error_recovery.json | 3 + .../ErrorRecoveryWizard.tsx | 13 ++- .../ErrorRecoveryFlows/RecoveryInProgress.tsx | 3 + .../RecoveryOptions/CancelRun.tsx | 76 +++++++++++++++ .../RecoveryOptions/ResumeRun.tsx | 2 +- .../__tests__/CancelRun.test.tsx | 93 +++++++++++++++++++ .../RecoveryOptions/index.ts | 1 + .../shared/RecoveryFooterButtons.tsx | 13 +-- .../__tests__/ErrorRecoveryFlows.test.tsx | 15 ++- .../__tests__/ErrorRecoveryWizard.test.tsx | 14 +++ .../__tests__/RecoveryInProgress.test.tsx | 14 +++ .../__tests__/useRecoveryCommands.test.ts | 22 ++++- .../organisms/ErrorRecoveryFlows/constants.ts | 15 ++- .../organisms/ErrorRecoveryFlows/index.tsx | 18 +++- app/src/organisms/ErrorRecoveryFlows/types.ts | 3 + .../ErrorRecoveryFlows/useRecoveryCommands.ts | 12 ++- 16 files changed, 292 insertions(+), 25 deletions(-) create mode 100644 app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/CancelRun.tsx create mode 100644 app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/CancelRun.test.tsx diff --git a/app/src/assets/localization/en/error_recovery.json b/app/src/assets/localization/en/error_recovery.json index 6ba2520aebb..1f634c3b370 100644 --- a/app/src/assets/localization/en/error_recovery.json +++ b/app/src/assets/localization/en/error_recovery.json @@ -1,7 +1,9 @@ { + "are_you_sure_you_want_to_cancel": "Are you sure you want to cancel?", "are_you_sure_you_want_to_resume": "Are you sure you want to resume?", "before_you_begin": "Before you begin", "cancel_run": "Cancel run", + "canceling_run": "Canceling run", "confirm": "Confirm", "continue": "Continue", "general_error": "General error", @@ -13,6 +15,7 @@ "resume": "Resume", "run_paused": "Run paused", "run_will_resume": "The run will resume from the point at which the error occurred. Take any necessary actions to correct the problem first. If the step is completed successfully, the protocol continues.", + "if_tips_are_attached": "If tips are attached, you can choose to blow out any aspirated liquid and drop tips before the run is terminated.", "stand_back": "Stand back, robot is in motion", "stand_back_resuming": "Stand back, resuming current step", "stand_back_retrying": "Stand back, retrying current command", diff --git a/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx b/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx index 55fbf5047b9..422256345f3 100644 --- a/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx @@ -11,9 +11,9 @@ import { } from '@opentrons/components' import { getIsOnDevice } from '../../redux/config' -import { getModalPortalEl } from '../../App/portal' +import { getTopPortalEl } from '../../App/portal' import { BeforeBeginning } from './BeforeBeginning' -import { SelectRecoveryOption, ResumeRun } from './RecoveryOptions' +import { SelectRecoveryOption, ResumeRun, CancelRun } from './RecoveryOptions' import { ErrorRecoveryHeader } from './ErrorRecoveryHeader' import { RecoveryInProgress } from './RecoveryInProgress' import { getErrorKind, useRouteUpdateActions } from './utils' @@ -80,7 +80,7 @@ function ErrorRecoveryComponent(props: RecoveryContentProps): JSX.Element { , - getModalPortalEl() + getTopPortalEl() ) } @@ -101,6 +101,10 @@ export function ErrorRecoveryContent(props: RecoveryContentProps): JSX.Element { return } + const buildCancelRun = (): JSX.Element => { + return + } + switch (props.recoveryMap.route) { case RECOVERY_MAP.BEFORE_BEGINNING.ROUTE: return buildBeforeBeginning() @@ -108,9 +112,12 @@ export function ErrorRecoveryContent(props: RecoveryContentProps): JSX.Element { return buildSelectRecoveryOption() case RECOVERY_MAP.RESUME.ROUTE: return buildResumeRun() + case RECOVERY_MAP.CANCEL_RUN.ROUTE: + return buildCancelRun() case RECOVERY_MAP.ROBOT_IN_MOTION.ROUTE: case RECOVERY_MAP.ROBOT_RESUMING.ROUTE: case RECOVERY_MAP.ROBOT_RETRYING_COMMAND.ROUTE: + case RECOVERY_MAP.ROBOT_CANCELING.ROUTE: return buildRecoveryInProgress() default: return buildSelectRecoveryOption() diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryInProgress.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryInProgress.tsx index 27123352933..94ad1cc64d1 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryInProgress.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryInProgress.tsx @@ -10,6 +10,7 @@ export function RecoveryInProgress({ recoveryMap, }: RecoveryContentProps): JSX.Element { const { + ROBOT_CANCELING, ROBOT_IN_MOTION, ROBOT_RESUMING, ROBOT_RETRYING_COMMAND, @@ -19,6 +20,8 @@ export function RecoveryInProgress({ const buildDescription = (): RobotMovingRoute => { switch (route) { + case ROBOT_CANCELING.ROUTE: + return t('canceling_run') case ROBOT_IN_MOTION.ROUTE: return t('stand_back') case ROBOT_RESUMING.ROUTE: diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/CancelRun.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/CancelRun.tsx new file mode 100644 index 00000000000..91ff0f7985a --- /dev/null +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/CancelRun.tsx @@ -0,0 +1,76 @@ +import * as React from 'react' +import { useTranslation } from 'react-i18next' + +import { + ALIGN_CENTER, + DIRECTION_COLUMN, + COLORS, + Flex, + Icon, + JUSTIFY_SPACE_BETWEEN, + SPACING, + StyledText, +} from '@opentrons/components' + +import { RECOVERY_MAP } from '../constants' +import { RecoveryFooterButtons } from './shared' + +import type { RecoveryContentProps } from '../types' + +export function CancelRun({ + isOnDevice, + routeUpdateActions, + recoveryCommands, +}: RecoveryContentProps): JSX.Element | null { + const { ROBOT_CANCELING } = RECOVERY_MAP + const { t } = useTranslation('error_recovery') + + const { cancelRun } = recoveryCommands + const { goBackPrevStep, setRobotInMotion } = routeUpdateActions + + const primaryBtnOnClick = (): Promise => { + return setRobotInMotion(true, ROBOT_CANCELING.ROUTE).then(() => cancelRun()) + } + + if (isOnDevice) { + return ( + + + + + {t('are_you_sure_you_want_to_cancel')} + + + {t('if_tips_are_attached')} + + + + + ) + } else { + return null + } +} diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ResumeRun.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ResumeRun.tsx index d702db177f9..3fb935747e0 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ResumeRun.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ResumeRun.tsx @@ -28,7 +28,7 @@ export function ResumeRun({ const { goBackPrevStep, setRobotInMotion } = routeUpdateActions const primaryBtnOnClick = (): Promise => { - return setRobotInMotion(true, ROBOT_RETRYING_COMMAND.ROUTE) // Show the "retrying" motion screen while exiting ER. + return setRobotInMotion(true, ROBOT_RETRYING_COMMAND.ROUTE) .then(() => retryFailedCommand()) .then(() => resumeRun()) } diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/CancelRun.test.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/CancelRun.test.tsx new file mode 100644 index 00000000000..24a8c0c29ed --- /dev/null +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/CancelRun.test.tsx @@ -0,0 +1,93 @@ +import * as React from 'react' +import { vi, describe, it, expect, beforeEach } from 'vitest' +import { screen, fireEvent, waitFor } from '@testing-library/react' + +import { renderWithProviders } from '../../../../__testing-utils__' +import { i18n } from '../../../../i18n' +import { CancelRun } from '../CancelRun' +import { RECOVERY_MAP, ERROR_KINDS } from '../../constants' + +import type { Mock } from 'vitest' + +const render = (props: React.ComponentProps) => { + return renderWithProviders(, { + i18nInstance: i18n, + })[0] +} + +describe('RecoveryFooterButtons', () => { + const { CANCEL_RUN, ROBOT_CANCELING } = RECOVERY_MAP + let props: React.ComponentProps + let mockGoBackPrevStep: Mock + + beforeEach(() => { + mockGoBackPrevStep = vi.fn() + const mockRouteUpdateActions = { goBackPrevStep: mockGoBackPrevStep } as any + + props = { + isOnDevice: true, + recoveryCommands: {} as any, + failedCommand: {} as any, + errorKind: ERROR_KINDS.GENERAL_ERROR, + routeUpdateActions: mockRouteUpdateActions, + recoveryMap: { + route: CANCEL_RUN.ROUTE, + step: CANCEL_RUN.STEPS.CONFIRM_CANCEL, + }, + } + }) + + it('renders appropriate copy and click behavior', async () => { + render(props) + + screen.getByText('Are you sure you want to cancel?') + screen.queryByText( + 'If tips are attached, you can choose to blowout any aspirated liquid and drop tips before the run is terminated.' + ) + + const secondaryBtn = screen.getByRole('button', { name: 'Go back' }) + + fireEvent.click(secondaryBtn) + + expect(mockGoBackPrevStep).toHaveBeenCalled() + }) + + it('should call commands in the correct order for the primaryOnClick callback', async () => { + const setRobotInMotionMock = vi.fn(() => Promise.resolve()) + const cancelRunMock = vi.fn(() => Promise.resolve()) + + const mockRecoveryCommands = { + cancelRun: cancelRunMock, + } as any + + const mockRouteUpdateActions = { + setRobotInMotion: setRobotInMotionMock, + } as any + + render({ + ...props, + recoveryCommands: mockRecoveryCommands, + routeUpdateActions: mockRouteUpdateActions, + }) + + const primaryBtn = screen.getByRole('button', { name: 'Confirm' }) + fireEvent.click(primaryBtn) + + await waitFor(() => { + expect(setRobotInMotionMock).toHaveBeenCalledTimes(1) + }) + await waitFor(() => { + expect(setRobotInMotionMock).toHaveBeenCalledWith( + true, + ROBOT_CANCELING.ROUTE + ) + }) + await waitFor(() => { + expect(cancelRunMock).toHaveBeenCalledTimes(1) + }) + + expect(setRobotInMotionMock.mock.invocationCallOrder[0]).toBeLessThan( + cancelRunMock.mock.invocationCallOrder[0] + ) + }) +}) diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/index.ts b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/index.ts index c39d9e883c6..12f638458ef 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/index.ts +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/index.ts @@ -1,2 +1,3 @@ export { SelectRecoveryOption } from './SelectRecoveryOption' export { ResumeRun } from './ResumeRun' +export { CancelRun } from './CancelRun' diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/shared/RecoveryFooterButtons.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/shared/RecoveryFooterButtons.tsx index 64264630c2d..63e167fc355 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/shared/RecoveryFooterButtons.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/shared/RecoveryFooterButtons.tsx @@ -4,16 +4,11 @@ import { useTranslation } from 'react-i18next' import { ALIGN_CENTER, Flex, - JUSTIFY_CENTER, JUSTIFY_SPACE_BETWEEN, SPACING, } from '@opentrons/components' import { SmallButton } from '../../../../atoms/buttons' -import { - NON_SANCTIONED_RECOVERY_COLOR_STYLE_PRIMARY, - NON_SANCTIONED_RECOVERY_COLOR_STYLE_SECONDARY, -} from '../../constants' interface RecoveryOptionProps { isOnDevice: boolean @@ -39,20 +34,14 @@ export function RecoveryFooterButtons({ gridGap={SPACING.spacing8} > diff --git a/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryFlows.test.tsx b/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryFlows.test.tsx index d44e3d4b333..2c3e8848a56 100644 --- a/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryFlows.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryFlows.test.tsx @@ -5,6 +5,7 @@ import { screen, renderHook, act } from '@testing-library/react' import { RUN_STATUS_AWAITING_RECOVERY, RUN_STATUS_RUNNING, + RUN_STATUS_STOP_REQUESTED, } from '@opentrons/api-client' import { renderWithProviders } from '../../../__testing-utils__' @@ -33,7 +34,7 @@ describe('useErrorRecovery', () => { expect(result.current.isERActive).toBe(false) }) - it('should toggle the value of isEREnabled properly', () => { + it('should toggle the value of isEREnabled properly when the run status is valid', () => { const { result } = renderHook(() => useErrorRecoveryFlows('MOCK_ID', RUN_STATUS_AWAITING_RECOVERY) ) @@ -48,9 +49,19 @@ describe('useErrorRecovery', () => { }) expect(result.current.isERActive).toBe(false) + + const { result: resultStopRequested } = renderHook(() => + useErrorRecoveryFlows('MOCK_ID', RUN_STATUS_STOP_REQUESTED) + ) + + act(() => { + resultStopRequested.current.toggleER() + }) + + expect(resultStopRequested.current.isERActive).toBe(true) }) - it('should disable error recovery when runStatus is not "awaiting-recovery"', () => { + it('should disable error recovery when runStatus is not a valid ER run status', () => { const { result, rerender } = renderHook( (runStatus: RunStatus) => useErrorRecoveryFlows('MOCK_ID', runStatus), { diff --git a/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryWizard.test.tsx b/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryWizard.test.tsx index 496fd676b9c..e6f77dd4c80 100644 --- a/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryWizard.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryWizard.test.tsx @@ -31,6 +31,7 @@ describe('ErrorRecoveryContent', () => { OPTION_SELECTION, BEFORE_BEGINNING, RESUME, + ROBOT_CANCELING, ROBOT_RESUMING, ROBOT_IN_MOTION, ROBOT_RETRYING_COMMAND, @@ -92,6 +93,19 @@ describe('ErrorRecoveryContent', () => { screen.getByText('MOCK_RESUME_RUN') }) + it(`returns RecoveryInProgressModal when the route is ${ROBOT_CANCELING.ROUTE}`, () => { + props = { + ...props, + recoveryMap: { + ...props.recoveryMap, + route: ROBOT_CANCELING.ROUTE, + }, + } + render(props) + + screen.getByText('MOCK_IN_PROGRESS') + }) + it(`returns RecoveryInProgressModal when the route is ${ROBOT_IN_MOTION.ROUTE}`, () => { props = { ...props, diff --git a/app/src/organisms/ErrorRecoveryFlows/__tests__/RecoveryInProgress.test.tsx b/app/src/organisms/ErrorRecoveryFlows/__tests__/RecoveryInProgress.test.tsx index a280a270970..fc9523f4efb 100644 --- a/app/src/organisms/ErrorRecoveryFlows/__tests__/RecoveryInProgress.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/__tests__/RecoveryInProgress.test.tsx @@ -15,6 +15,7 @@ const render = (props: React.ComponentProps) => { describe('RecoveryInProgress', () => { const { + ROBOT_CANCELING, ROBOT_IN_MOTION, ROBOT_RESUMING, ROBOT_RETRYING_COMMAND, @@ -66,4 +67,17 @@ describe('RecoveryInProgress', () => { screen.getByText('Stand back, retrying current command') }) + + it(`renders appropriate copy when the route is ${ROBOT_CANCELING.ROUTE}`, () => { + props = { + ...props, + recoveryMap: { + route: ROBOT_CANCELING.ROUTE, + step: ROBOT_CANCELING.STEPS.CANCELING, + }, + } + render(props) + + screen.getByText('Canceling run') + }) }) diff --git a/app/src/organisms/ErrorRecoveryFlows/__tests__/useRecoveryCommands.test.ts b/app/src/organisms/ErrorRecoveryFlows/__tests__/useRecoveryCommands.test.ts index 801b5785b7e..26d654f9cef 100644 --- a/app/src/organisms/ErrorRecoveryFlows/__tests__/useRecoveryCommands.test.ts +++ b/app/src/organisms/ErrorRecoveryFlows/__tests__/useRecoveryCommands.test.ts @@ -1,7 +1,10 @@ import { vi, it, describe, expect, beforeEach } from 'vitest' import { renderHook, act } from '@testing-library/react' -import { useResumeRunFromRecoveryMutation } from '@opentrons/react-api-client' +import { + useResumeRunFromRecoveryMutation, + useStopRunMutation, +} from '@opentrons/react-api-client' import { useChainRunCommands } from '../../../resources/runs' import { @@ -21,12 +24,16 @@ const mockRunId = '123' describe('useRecoveryCommands', () => { const mockResumeRunFromRecovery = vi.fn() + const mockStopRun = vi.fn() const mockChainRunCommands = vi.fn().mockResolvedValue([]) beforeEach(() => { vi.mocked(useResumeRunFromRecoveryMutation).mockReturnValue({ resumeRunFromRecovery: mockResumeRunFromRecovery, } as any) + vi.mocked(useStopRunMutation).mockReturnValue({ + stopRun: mockStopRun, + } as any) vi.mocked(useChainRunCommands).mockReturnValue({ chainRunCommands: mockChainRunCommands, } as any) @@ -86,6 +93,19 @@ describe('useRecoveryCommands', () => { expect(mockResumeRunFromRecovery).toHaveBeenCalledWith(mockRunId) }) + it('should call cancelRun with runId', () => { + const { result } = renderHook(() => + useRecoveryCommands({ + runId: mockRunId, + failedCommand: mockFailedCommand, + }) + ) + + result.current.cancelRun() + + expect(mockStopRun).toHaveBeenCalledWith(mockRunId) + }) + it('should call homePipetteZAxes with the appropriate command', async () => { const { result } = renderHook(() => useRecoveryCommands({ diff --git a/app/src/organisms/ErrorRecoveryFlows/constants.ts b/app/src/organisms/ErrorRecoveryFlows/constants.ts index e0da763d076..bf428b2aa08 100644 --- a/app/src/organisms/ErrorRecoveryFlows/constants.ts +++ b/app/src/organisms/ErrorRecoveryFlows/constants.ts @@ -17,7 +17,10 @@ export const RECOVERY_MAP = { RECOVERY_DESCRIPTION: 'recovery-description', }, }, - CANCEL_RUN: { ROUTE: 'cancel-run', STEPS: {} }, + CANCEL_RUN: { + ROUTE: 'cancel-run', + STEPS: { CONFIRM_CANCEL: 'confirm-cancel' }, + }, DROP_TIP: { ROUTE: 'drop-tip', STEPS: {} }, IGNORE_AND_RESUME: { ROUTE: 'ignore-and-resume', STEPS: {} }, REFILL_AND_RESUME: { ROUTE: 'refill-and-resume', STEPS: {} }, @@ -25,6 +28,12 @@ export const RECOVERY_MAP = { ROUTE: 'resume', STEPS: { CONFIRM_RESUME: 'confirm-resume' }, }, + ROBOT_CANCELING: { + ROUTE: 'robot-cancel-run', + STEPS: { + CANCELING: 'canceling', + }, + }, ROBOT_IN_MOTION: { ROUTE: 'robot-in-motion', STEPS: { @@ -53,6 +62,7 @@ const { BEFORE_BEGINNING, OPTION_SELECTION, RESUME, + ROBOT_CANCELING, ROBOT_RESUMING, ROBOT_IN_MOTION, ROBOT_RETRYING_COMMAND, @@ -67,13 +77,14 @@ export const STEP_ORDER: StepOrder = { [BEFORE_BEGINNING.ROUTE]: [BEFORE_BEGINNING.STEPS.RECOVERY_DESCRIPTION], [OPTION_SELECTION.ROUTE]: [OPTION_SELECTION.STEPS.SELECT], [RESUME.ROUTE]: [RESUME.STEPS.CONFIRM_RESUME], + [ROBOT_CANCELING.ROUTE]: [ROBOT_CANCELING.STEPS.CANCELING], [ROBOT_IN_MOTION.ROUTE]: [ROBOT_IN_MOTION.STEPS.IN_MOTION], [ROBOT_RESUMING.ROUTE]: [ROBOT_RESUMING.STEPS.RESUMING], [ROBOT_RETRYING_COMMAND.ROUTE]: [ROBOT_RETRYING_COMMAND.STEPS.RETRYING], [DROP_TIP.ROUTE]: [], [REFILL_AND_RESUME.ROUTE]: [], [IGNORE_AND_RESUME.ROUTE]: [], - [CANCEL_RUN.ROUTE]: [], + [CANCEL_RUN.ROUTE]: [CANCEL_RUN.STEPS.CONFIRM_CANCEL], } export const INVALID = 'INVALID' as const diff --git a/app/src/organisms/ErrorRecoveryFlows/index.tsx b/app/src/organisms/ErrorRecoveryFlows/index.tsx index b7da1f4caaf..2a41f2fca2f 100644 --- a/app/src/organisms/ErrorRecoveryFlows/index.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/index.tsx @@ -1,6 +1,9 @@ import * as React from 'react' -import { RUN_STATUS_AWAITING_RECOVERY } from '@opentrons/api-client' +import { + RUN_STATUS_AWAITING_RECOVERY, + RUN_STATUS_STOP_REQUESTED, +} from '@opentrons/api-client' import { ErrorRecoveryWizard } from './ErrorRecoveryWizard' import { useCurrentlyFailedRunCommand } from './utils' @@ -8,6 +11,11 @@ import { useCurrentlyFailedRunCommand } from './utils' import type { RunStatus } from '@opentrons/api-client' import type { FailedCommand } from './types' +const VALID_ER_RUN_STATUSES: RunStatus[] = [ + RUN_STATUS_AWAITING_RECOVERY, + RUN_STATUS_STOP_REQUESTED, +] + interface UseErrorRecoveryResult { isERActive: boolean failedCommand: FailedCommand | null @@ -25,9 +33,13 @@ export function useErrorRecoveryFlows( setIsERActive(!isERActive) } - // Because multiple ER flows may occur per run, disable ER when the status is not "awaiting-recovery." + // 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. React.useEffect(() => { - if (isERActive && runStatus !== RUN_STATUS_AWAITING_RECOVERY) { + const isValidRunStatus = + runStatus != null && VALID_ER_RUN_STATUSES.includes(runStatus) + + if (isERActive && !isValidRunStatus) { setIsERActive(false) } }, [isERActive, runStatus]) diff --git a/app/src/organisms/ErrorRecoveryFlows/types.ts b/app/src/organisms/ErrorRecoveryFlows/types.ts index 794b36e679e..00da630fedb 100644 --- a/app/src/organisms/ErrorRecoveryFlows/types.ts +++ b/app/src/organisms/ErrorRecoveryFlows/types.ts @@ -10,6 +10,7 @@ export type RobotMovingRoute = | typeof RECOVERY_MAP['ROBOT_IN_MOTION']['ROUTE'] | typeof RECOVERY_MAP['ROBOT_RESUMING']['ROUTE'] | typeof RECOVERY_MAP['ROBOT_RETRYING_COMMAND']['ROUTE'] + | typeof RECOVERY_MAP['ROBOT_CANCELING']['ROUTE'] export type ErrorKind = keyof typeof ERROR_KINDS interface RecoveryMapDetails { @@ -27,6 +28,7 @@ type RecoveryStep< K extends keyof RecoveryMap > = RecoveryMap[K]['STEPS'][keyof RecoveryMap[K]['STEPS']] +type RobotCancellingRunStep = RecoveryStep<'ROBOT_CANCELING'> type RobotInMotionStep = RecoveryStep<'ROBOT_IN_MOTION'> type RobotResumingStep = RecoveryStep<'ROBOT_RESUMING'> type RobotRetryingCommandStep = RecoveryStep<'ROBOT_RETRYING_COMMAND'> @@ -49,6 +51,7 @@ export type RouteStep = | ResumeStep | OptionSelectionStep | RefillAndResumeStep + | RobotCancellingRunStep export interface IRecoveryMap { route: RecoveryRoute diff --git a/app/src/organisms/ErrorRecoveryFlows/useRecoveryCommands.ts b/app/src/organisms/ErrorRecoveryFlows/useRecoveryCommands.ts index 78cab04f2d4..59956e0f1fb 100644 --- a/app/src/organisms/ErrorRecoveryFlows/useRecoveryCommands.ts +++ b/app/src/organisms/ErrorRecoveryFlows/useRecoveryCommands.ts @@ -1,6 +1,9 @@ import * as React from 'react' -import { useResumeRunFromRecoveryMutation } from '@opentrons/react-api-client' +import { + useResumeRunFromRecoveryMutation, + useStopRunMutation, +} from '@opentrons/react-api-client' import { useChainRunCommands } from '../../resources/runs' @@ -14,6 +17,7 @@ interface UseRecoveryCommandsParams { } export interface UseRecoveryCommandsResult { resumeRun: () => void + cancelRun: () => void retryFailedCommand: () => Promise homePipetteZAxes: () => Promise } @@ -24,6 +28,7 @@ export function useRecoveryCommands({ }: UseRecoveryCommandsParams): UseRecoveryCommandsResult { const { chainRunCommands } = useChainRunCommands(runId, failedCommand?.id) const { resumeRunFromRecovery } = useResumeRunFromRecoveryMutation() + const { stopRun } = useStopRunMutation() const chainRunRecoveryCommands = React.useCallback( ( @@ -54,8 +59,13 @@ export function useRecoveryCommands({ resumeRunFromRecovery(runId) }, [runId, resumeRunFromRecovery]) + const cancelRun = React.useCallback((): void => { + stopRun(runId) + }, [runId]) + return { resumeRun, + cancelRun, retryFailedCommand, homePipetteZAxes, } From 7fc230f227691ed788688b837e8d34a144d70dbb Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Wed, 22 May 2024 10:13:55 -0400 Subject: [PATCH 08/17] chore(api): raise EnumeratedError for failed status check (#15241) If you did a motor_engaged() query and the node didn't respond, you'd get a KeyError wrapped in a 4000 error, which isn't correct or useful. Make that a 1000 error instead. --- .../hardware_control/backends/ot3controller.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/api/src/opentrons/hardware_control/backends/ot3controller.py b/api/src/opentrons/hardware_control/backends/ot3controller.py index 4dd1e814b8e..1b90e086dca 100644 --- a/api/src/opentrons/hardware_control/backends/ot3controller.py +++ b/api/src/opentrons/hardware_control/backends/ot3controller.py @@ -192,6 +192,8 @@ FirmwareUpdateRequiredError, FailedGripperPickupError, LiquidNotFoundError, + CommunicationError, + PythonException, ) from .subsystem_manager import SubsystemManager @@ -1186,7 +1188,14 @@ async def update_engaged_axes(self) -> None: async def is_motor_engaged(self, axis: Axis) -> bool: node = axis_to_node(axis) result = await get_motor_enabled(self._messenger, {node}) - engaged = result[node] + try: + engaged = result[node] + except KeyError as ke: + raise CommunicationError( + message=f"No response from {node.name} for motor engagement query", + detail={"node": node.name}, + wrapping=[PythonException(ke)], + ) from ke self._engaged_axes.update({axis: engaged}) return engaged From 7b91a08304f799e80fc6f49301994c1595717e42 Mon Sep 17 00:00:00 2001 From: Jethary Rader <66035149+jerader@users.noreply.github.com> Date: Wed, 22 May 2024 10:21:03 -0400 Subject: [PATCH 09/17] fix(shared-data): raise uiMaxFlowRate values to match the default blowout and dispense (#15238) closes RQA-2722 A few of the `uiMaxFlowRate` values were not high enough to account for the default `blowout` and `dispense` flow rates. Rather than changing those flow rates, I raised the `uiMaxFlowRate`s for those pipette models. This required me to modify the test to expect the failures for those models. --- shared-data/js/__tests__/pipettes.test.ts | 2 +- .../liquid/eight_channel/p50/default/3_4.json | 2 +- .../liquid/eight_channel/p50/default/3_5.json | 2 +- .../single_channel/p50/default/3_4.json | 2 +- .../single_channel/p50/default/3_5.json | 2 +- .../single_channel/p50/default/3_6.json | 2 +- .../pipette/test_max_flow_rates_per_volume.py | 35 +++++++++++++------ 7 files changed, 31 insertions(+), 16 deletions(-) diff --git a/shared-data/js/__tests__/pipettes.test.ts b/shared-data/js/__tests__/pipettes.test.ts index 14b3b417a8f..6fcc519f2d3 100644 --- a/shared-data/js/__tests__/pipettes.test.ts +++ b/shared-data/js/__tests__/pipettes.test.ts @@ -158,7 +158,7 @@ describe('pipette data accessors', () => { minVolume: 5, supportedTips: { t50: { - uiMaxFlowRate: 47, + uiMaxFlowRate: 57, aspirate: { default: { 1: expect.anything(), diff --git a/shared-data/pipette/definitions/2/liquid/eight_channel/p50/default/3_4.json b/shared-data/pipette/definitions/2/liquid/eight_channel/p50/default/3_4.json index 32131ee1982..f1c15ddae49 100644 --- a/shared-data/pipette/definitions/2/liquid/eight_channel/p50/default/3_4.json +++ b/shared-data/pipette/definitions/2/liquid/eight_channel/p50/default/3_4.json @@ -2,7 +2,7 @@ "$otSharedSchema": "#/pipette/schemas/2/pipetteLiquidPropertiesSchema.json", "supportedTips": { "t50": { - "uiMaxFlowRate": 46.8, + "uiMaxFlowRate": 57, "defaultAspirateFlowRate": { "default": 35, "valuesByApiLevel": { "2.14": 35 } diff --git a/shared-data/pipette/definitions/2/liquid/eight_channel/p50/default/3_5.json b/shared-data/pipette/definitions/2/liquid/eight_channel/p50/default/3_5.json index ca2a48db274..80763fd4b6c 100644 --- a/shared-data/pipette/definitions/2/liquid/eight_channel/p50/default/3_5.json +++ b/shared-data/pipette/definitions/2/liquid/eight_channel/p50/default/3_5.json @@ -2,7 +2,7 @@ "$otSharedSchema": "#/pipette/schemas/2/pipetteLiquidPropertiesSchema.json", "supportedTips": { "t50": { - "uiMaxFlowRate": 46.7, + "uiMaxFlowRate": 57, "defaultAspirateFlowRate": { "default": 35, "valuesByApiLevel": { "2.14": 35 } diff --git a/shared-data/pipette/definitions/2/liquid/single_channel/p50/default/3_4.json b/shared-data/pipette/definitions/2/liquid/single_channel/p50/default/3_4.json index 464eb213798..24da0837138 100644 --- a/shared-data/pipette/definitions/2/liquid/single_channel/p50/default/3_4.json +++ b/shared-data/pipette/definitions/2/liquid/single_channel/p50/default/3_4.json @@ -2,7 +2,7 @@ "$otSharedSchema": "#/pipette/schemas/2/pipetteLiquidPropertiesSchema.json", "supportedTips": { "t50": { - "uiMaxFlowRate": 46.3, + "uiMaxFlowRate": 57, "defaultAspirateFlowRate": { "default": 35, "valuesByApiLevel": { "2.14": 35 } diff --git a/shared-data/pipette/definitions/2/liquid/single_channel/p50/default/3_5.json b/shared-data/pipette/definitions/2/liquid/single_channel/p50/default/3_5.json index 2fca659b070..0077dcf4c76 100644 --- a/shared-data/pipette/definitions/2/liquid/single_channel/p50/default/3_5.json +++ b/shared-data/pipette/definitions/2/liquid/single_channel/p50/default/3_5.json @@ -2,7 +2,7 @@ "$otSharedSchema": "#/pipette/schemas/2/pipetteLiquidPropertiesSchema.json", "supportedTips": { "t50": { - "uiMaxFlowRate": 47, + "uiMaxFlowRate": 57, "defaultAspirateFlowRate": { "default": 35, "valuesByApiLevel": { "2.14": 35 } diff --git a/shared-data/pipette/definitions/2/liquid/single_channel/p50/default/3_6.json b/shared-data/pipette/definitions/2/liquid/single_channel/p50/default/3_6.json index 2fca659b070..0077dcf4c76 100644 --- a/shared-data/pipette/definitions/2/liquid/single_channel/p50/default/3_6.json +++ b/shared-data/pipette/definitions/2/liquid/single_channel/p50/default/3_6.json @@ -2,7 +2,7 @@ "$otSharedSchema": "#/pipette/schemas/2/pipetteLiquidPropertiesSchema.json", "supportedTips": { "t50": { - "uiMaxFlowRate": 47, + "uiMaxFlowRate": 57, "defaultAspirateFlowRate": { "default": 35, "valuesByApiLevel": { "2.14": 35 } diff --git a/shared-data/python/tests/pipette/test_max_flow_rates_per_volume.py b/shared-data/python/tests/pipette/test_max_flow_rates_per_volume.py index ff731ec0e3c..45ce013d9d8 100644 --- a/shared-data/python/tests/pipette/test_max_flow_rates_per_volume.py +++ b/shared-data/python/tests/pipette/test_max_flow_rates_per_volume.py @@ -75,14 +75,29 @@ def test_max_flow_rates_per_volume(pipette: PipetteModel, action: str) -> None: pipette_model_version.pipette_channels, pipette_model_version.pipette_version, ) + + pipette_model_version_str = f"{pipette_model_version}" + for liquid_name, liquid_properties in definition.liquid_properties.items(): - for ( - tip_type, - supported_tip, - ) in liquid_properties.supported_tips.items(): - assert supported_tip.ui_max_flow_rate < _get_max_flow_rate_at_volume( - supported_tip.aspirate, pipette, liquid_properties.min_volume - ) - assert supported_tip.ui_max_flow_rate < _get_max_flow_rate_at_volume( - supported_tip.dispense, pipette, liquid_properties.min_volume - ) + for tip_type, supported_tip in liquid_properties.supported_tips.items(): + + """TODO: the following models do not pass the asserts since the uiMaxFlowRate was raised + to match the default blowout and dispense flowRates. uiMaxFlowRate will be reevaluated + in the future.""" + if not ( + pipette_model_version_str + in { + "p50_single_v3.4", + "p50_single_v3.5", + "p50_single_v3.6", + "p50_multi_v3.5", + "p50_multi_v3.4", + } + and liquid_properties.min_volume == 5.0 + ): + assert supported_tip.ui_max_flow_rate < _get_max_flow_rate_at_volume( + supported_tip.aspirate, pipette, liquid_properties.min_volume + ) + assert supported_tip.ui_max_flow_rate < _get_max_flow_rate_at_volume( + supported_tip.dispense, pipette, liquid_properties.min_volume + ) From 432e353107a3ceff07ca31226f1eefc7954b79cd Mon Sep 17 00:00:00 2001 From: Jethary Rader <66035149+jerader@users.noreply.github.com> Date: Wed, 22 May 2024 10:23:55 -0400 Subject: [PATCH 10/17] refactor(protocol-designer): dismissed warning now dismisses for duration of protocol (#15244) closes RQA-2756 --- .../protocol/8/doItAllV3MigratedToV8.json | 2 +- .../protocol/8/doItAllV4MigratedToV8.json | 2 +- .../protocol/8/doItAllV7MigratedToV8.json | 2 +- .../fixtures/protocol/8/doItAllV8.json | 2 +- .../protocol/8/example_1_1_0MigratedToV8.json | 2 +- .../fixtures/protocol/8/mix_8_0_0.json | 2 +- .../8/newAdvancedSettingsAndMultiTemp.json | 2 +- .../8/ninetySixChannelFullAndColumn.json | 2 +- .../src/components/alerts/Alerts.tsx | 4 - .../src/dismiss/__tests__/reducers.test.ts | 105 +++--------------- protocol-designer/src/dismiss/actions.ts | 3 - protocol-designer/src/dismiss/reducers.ts | 70 +++--------- protocol-designer/src/dismiss/selectors.ts | 39 ++----- .../__fixtures__/createFile/commonFields.ts | 4 +- .../src/load-file/migration/8_1_0.ts | 21 ++++ .../migration/utils/getLoadLiquidCommands.ts | 2 + .../top-selectors/timelineWarnings/index.ts | 7 +- 17 files changed, 78 insertions(+), 193 deletions(-) diff --git a/protocol-designer/fixtures/protocol/8/doItAllV3MigratedToV8.json b/protocol-designer/fixtures/protocol/8/doItAllV3MigratedToV8.json index c33b1764d40..a44442ee880 100644 --- a/protocol-designer/fixtures/protocol/8/doItAllV3MigratedToV8.json +++ b/protocol-designer/fixtures/protocol/8/doItAllV3MigratedToV8.json @@ -27,7 +27,7 @@ "opentrons/opentrons_96_tiprack_300ul/1" ] }, - "dismissedWarnings": { "form": {}, "timeline": {} }, + "dismissedWarnings": { "form": [], "timeline": [] }, "ingredients": { "0": { "name": "Water", diff --git a/protocol-designer/fixtures/protocol/8/doItAllV4MigratedToV8.json b/protocol-designer/fixtures/protocol/8/doItAllV4MigratedToV8.json index 5876d7bf450..5aa4cf659fc 100644 --- a/protocol-designer/fixtures/protocol/8/doItAllV4MigratedToV8.json +++ b/protocol-designer/fixtures/protocol/8/doItAllV4MigratedToV8.json @@ -27,7 +27,7 @@ "opentrons/opentrons_96_tiprack_300ul/1" ] }, - "dismissedWarnings": { "form": {}, "timeline": {} }, + "dismissedWarnings": { "form": [], "timeline": [] }, "ingredients": { "0": { "name": "Water", diff --git a/protocol-designer/fixtures/protocol/8/doItAllV7MigratedToV8.json b/protocol-designer/fixtures/protocol/8/doItAllV7MigratedToV8.json index 47fe0d1f860..cc197f448ce 100644 --- a/protocol-designer/fixtures/protocol/8/doItAllV7MigratedToV8.json +++ b/protocol-designer/fixtures/protocol/8/doItAllV7MigratedToV8.json @@ -30,7 +30,7 @@ "opentrons/opentrons_flex_96_filtertiprack_50ul/1" ] }, - "dismissedWarnings": { "form": {}, "timeline": {} }, + "dismissedWarnings": { "form": [], "timeline": [] }, "ingredients": { "0": { "name": "Water", diff --git a/protocol-designer/fixtures/protocol/8/doItAllV8.json b/protocol-designer/fixtures/protocol/8/doItAllV8.json index 61f1e788852..fa212494e19 100644 --- a/protocol-designer/fixtures/protocol/8/doItAllV8.json +++ b/protocol-designer/fixtures/protocol/8/doItAllV8.json @@ -27,7 +27,7 @@ "opentrons/opentrons_flex_96_tiprack_1000ul/1" ] }, - "dismissedWarnings": { "form": {}, "timeline": {} }, + "dismissedWarnings": { "form": [], "timeline": [] }, "ingredients": { "0": { "name": "h20", diff --git a/protocol-designer/fixtures/protocol/8/example_1_1_0MigratedToV8.json b/protocol-designer/fixtures/protocol/8/example_1_1_0MigratedToV8.json index c789d62cacc..369e5e7b464 100644 --- a/protocol-designer/fixtures/protocol/8/example_1_1_0MigratedToV8.json +++ b/protocol-designer/fixtures/protocol/8/example_1_1_0MigratedToV8.json @@ -30,7 +30,7 @@ "opentrons/tipone_96_tiprack_200ul/1" ] }, - "dismissedWarnings": { "form": {}, "timeline": {} }, + "dismissedWarnings": { "form": [], "timeline": [] }, "ingredients": { "0": { "name": "samples", diff --git a/protocol-designer/fixtures/protocol/8/mix_8_0_0.json b/protocol-designer/fixtures/protocol/8/mix_8_0_0.json index 6684720c7d9..079e1eb07ac 100644 --- a/protocol-designer/fixtures/protocol/8/mix_8_0_0.json +++ b/protocol-designer/fixtures/protocol/8/mix_8_0_0.json @@ -25,7 +25,7 @@ "pipetteTiprackAssignments": { "pipetteId": ["opentrons/opentrons_96_tiprack_10ul/1"] }, - "dismissedWarnings": { "form": {}, "timeline": {} }, + "dismissedWarnings": { "form": [], "timeline": [] }, "ingredients": {}, "ingredLocations": {}, "savedStepForms": { diff --git a/protocol-designer/fixtures/protocol/8/newAdvancedSettingsAndMultiTemp.json b/protocol-designer/fixtures/protocol/8/newAdvancedSettingsAndMultiTemp.json index 58325488533..1144f644f53 100644 --- a/protocol-designer/fixtures/protocol/8/newAdvancedSettingsAndMultiTemp.json +++ b/protocol-designer/fixtures/protocol/8/newAdvancedSettingsAndMultiTemp.json @@ -27,7 +27,7 @@ "opentrons/opentrons_flex_96_tiprack_50ul/1" ] }, - "dismissedWarnings": { "form": {}, "timeline": {} }, + "dismissedWarnings": { "form": [], "timeline": [] }, "ingredients": {}, "ingredLocations": {}, "savedStepForms": { diff --git a/protocol-designer/fixtures/protocol/8/ninetySixChannelFullAndColumn.json b/protocol-designer/fixtures/protocol/8/ninetySixChannelFullAndColumn.json index dc58d3219a2..d19277a5d35 100644 --- a/protocol-designer/fixtures/protocol/8/ninetySixChannelFullAndColumn.json +++ b/protocol-designer/fixtures/protocol/8/ninetySixChannelFullAndColumn.json @@ -27,7 +27,7 @@ "opentrons/opentrons_flex_96_tiprack_50ul/1" ] }, - "dismissedWarnings": { "form": {}, "timeline": {} }, + "dismissedWarnings": { "form": [], "timeline": [] }, "ingredients": {}, "ingredLocations": { "9bd16b50-4ae9-4cfd-8583-3378087e6a6c:opentrons/opentrons_flex_96_tiprack_50ul/1": {} diff --git a/protocol-designer/src/components/alerts/Alerts.tsx b/protocol-designer/src/components/alerts/Alerts.tsx index c593f5ab74a..30f70e09242 100644 --- a/protocol-designer/src/components/alerts/Alerts.tsx +++ b/protocol-designer/src/components/alerts/Alerts.tsx @@ -10,7 +10,6 @@ import { } from '../../dismiss' import { selectors as stepFormSelectors } from '../../step-forms' import { selectors as fileDataSelectors } from '../../file-data' -import { PRESAVED_STEP_ID } from '../../steplist' import { getVisibleFormWarnings, getVisibleFormErrors, @@ -152,15 +151,12 @@ const AlertsComponent = (props: Props): JSX.Element => { dispatch( dismissActions.dismissTimelineWarning({ type: dismissId, - stepId, }) ) } else { dispatch( dismissActions.dismissFormWarning({ type: dismissId, - // if stepId does not exist, assume it is a presaved step - stepId: stepId ?? PRESAVED_STEP_ID, }) ) } diff --git a/protocol-designer/src/dismiss/__tests__/reducers.test.ts b/protocol-designer/src/dismiss/__tests__/reducers.test.ts index b303be6ae67..3d64b109e8e 100644 --- a/protocol-designer/src/dismiss/__tests__/reducers.test.ts +++ b/protocol-designer/src/dismiss/__tests__/reducers.test.ts @@ -1,6 +1,5 @@ import { describe, it, expect, beforeEach } from 'vitest' import { _allReducers } from '../reducers' -import { PRESAVED_STEP_ID } from '../../steplist/types' import type { DismissedWarningState } from '../reducers' const { dismissedWarnings } = _allReducers @@ -8,7 +7,7 @@ const { dismissedWarnings } = _allReducers let initialState: DismissedWarningState beforeEach(() => { - initialState = { form: {}, timeline: {} } + initialState = { form: [], timeline: [] } }) describe('dismissedWarnings reducer', () => { @@ -18,12 +17,11 @@ describe('dismissedWarnings reducer', () => { type: 'DISMISS_FORM_WARNING', payload: { type: 'BELOW_PIPETTE_MINIMUM_VOLUME', - stepId: 'someStepId', }, } expect(dismissedWarnings(state, action)).toEqual({ - form: { someStepId: ['BELOW_PIPETTE_MINIMUM_VOLUME'] }, - timeline: {}, + form: ['BELOW_PIPETTE_MINIMUM_VOLUME'], + timeline: [], }) }) @@ -33,14 +31,11 @@ describe('dismissedWarnings reducer', () => { type: 'DISMISS_FORM_WARNING', payload: { type: 'BELOW_PIPETTE_MINIMUM_VOLUME', - stepId: PRESAVED_STEP_ID, }, } expect(dismissedWarnings(state, action)).toEqual({ - form: { - [PRESAVED_STEP_ID]: ['BELOW_PIPETTE_MINIMUM_VOLUME'], - }, - timeline: {}, + form: ['BELOW_PIPETTE_MINIMUM_VOLUME'], + timeline: [], }) }) @@ -50,12 +45,11 @@ describe('dismissedWarnings reducer', () => { type: 'DISMISS_TIMELINE_WARNING', payload: { type: 'ASPIRATE_MORE_THAN_WELL_CONTENTS', - stepId: 'someStepId', }, } expect(dismissedWarnings(state, action)).toEqual({ - form: {}, - timeline: { someStepId: ['ASPIRATE_MORE_THAN_WELL_CONTENTS'] }, + form: [], + timeline: ['ASPIRATE_MORE_THAN_WELL_CONTENTS'], }) }) @@ -65,84 +59,11 @@ describe('dismissedWarnings reducer', () => { type: 'DISMISS_TIMELINE_WARNING', payload: { type: 'ASPIRATE_MORE_THAN_WELL_CONTENTS', - stepId: PRESAVED_STEP_ID, - }, - } - expect(dismissedWarnings(state, action)).toEqual({ - form: {}, - timeline: { - [PRESAVED_STEP_ID]: ['ASPIRATE_MORE_THAN_WELL_CONTENTS'], - }, - }) - }) - - it('should forget all warnings for a form upon DELETE_STEP', () => { - const state = { - form: { - otherStepId: ['whatever_form'], - someStepId: ['BELOW_PIPETTE_MINIMUM_VOLUME'], - }, - timeline: { - otherStepId: ['whatever_timeline'], - someStepId: ['ASPIRATE_MORE_THAN_WELL_CONTENTS'], - }, - } - - const action = { - type: 'DELETE_STEP', - payload: 'someStepId', - } - - expect(dismissedWarnings(state, action)).toEqual({ - form: { otherStepId: ['whatever_form'] }, - timeline: { otherStepId: ['whatever_timeline'] }, - }) - }) - - it('should forget all warnings for multiple forms upon DELETE_MULTIPLE_STEPS', () => { - const state = { - form: { - firstStepId: ['firstStepId form warning'], - secondStepId: ['secondStepId form warning'], - thirdStepId: ['thirdStepId form warning'], - }, - timeline: { - firstStepId: ['firstStepId timeline warning'], - secondStepId: ['secondStepId timeline warning'], - thirdStepId: ['thirdStepId timeline warning'], }, } - - const action = { - type: 'DELETE_MULTIPLE_STEPS', - payload: ['secondStepId', 'firstStepId'], - } - - expect(dismissedWarnings(state, action)).toEqual({ - form: { thirdStepId: ['thirdStepId form warning'] }, - timeline: { thirdStepId: ['thirdStepId timeline warning'] }, - }) - }) - - it('should forget all warnings for an unsaved form upon CANCEL_STEP_FORM', () => { - const state = { - form: { - otherStepId: ['whatever_form'], - [PRESAVED_STEP_ID]: ['BELOW_PIPETTE_MINIMUM_VOLUME'], - }, - timeline: { - otherStepId: ['whatever_timeline'], - [PRESAVED_STEP_ID]: ['ASPIRATE_MORE_THAN_WELL_CONTENTS'], - }, - } - const action = { - type: 'CANCEL_STEP_FORM', - payload: null, - } - expect(dismissedWarnings(state, action)).toEqual({ - form: { otherStepId: ['whatever_form'] }, - timeline: { otherStepId: ['whatever_timeline'] }, + form: [], + timeline: ['ASPIRATE_MORE_THAN_WELL_CONTENTS'], }) }) @@ -156,8 +77,8 @@ describe('dismissedWarnings reducer', () => { version: '5.0.1', data: { dismissedWarnings: { - form: { someStepId: ['whatever_form'] }, - timeline: { someStepId: ['whatever_timeline'] }, + form: ['whatever_form'], + timeline: ['whatever_timeline'], }, }, }, @@ -165,8 +86,8 @@ describe('dismissedWarnings reducer', () => { }, } expect(dismissedWarnings(initialState, action)).toEqual({ - form: { someStepId: ['whatever_form'] }, - timeline: { someStepId: ['whatever_timeline'] }, + form: ['whatever_form'], + timeline: ['whatever_timeline'], }) }) }) diff --git a/protocol-designer/src/dismiss/actions.ts b/protocol-designer/src/dismiss/actions.ts index 09f2c5a33c7..9708eba07d4 100644 --- a/protocol-designer/src/dismiss/actions.ts +++ b/protocol-designer/src/dismiss/actions.ts @@ -1,10 +1,7 @@ -import type { StepIdType } from '../form-types' - export interface DismissAction { type: ActionType payload: { type: string - stepId: StepIdType } } diff --git a/protocol-designer/src/dismiss/reducers.ts b/protocol-designer/src/dismiss/reducers.ts index 5b3cec94365..af05a0cc004 100644 --- a/protocol-designer/src/dismiss/reducers.ts +++ b/protocol-designer/src/dismiss/reducers.ts @@ -1,27 +1,29 @@ import { combineReducers } from 'redux' import { handleActions } from 'redux-actions' -import omit from 'lodash/omit' import { getPDMetadata } from '../file-types' -import { PRESAVED_STEP_ID } from '../steplist/types' import type { Reducer } from 'redux' -import type { DismissFormWarning, DismissTimelineWarning } from './actions' import type { BaseState, Action } from '../types' import type { LoadFileAction } from '../load-file' -import type { - CancelStepFormAction, - DeleteStepAction, - DeleteMultipleStepsAction, -} from '../steplist/actions' import type { StepIdType } from '../form-types' +import type { DismissFormWarning, DismissTimelineWarning } from './actions' + export type WarningType = string -export type DismissedWarningsAllSteps = Record< + +export interface DismissedWarningState { + form: WarningType[] + timeline: WarningType[] +} + +// these legacy types are used for the migration 8_1_0 +type LegacyDismissedWarningsAllSteps = Record< StepIdType, WarningType[] | null | undefined > -export interface DismissedWarningState { - form: DismissedWarningsAllSteps - timeline: DismissedWarningsAllSteps +export interface LegacyDismissedWarningState { + form: LegacyDismissedWarningsAllSteps + timeline: LegacyDismissedWarningsAllSteps } + // @ts-expect-error(sa, 2021-6-10): cannot use string literals as action type // TODO IMMEDIATELY: refactor this to the old fashioned way if we cannot have type safety: https://github.com/redux-utilities/redux-actions/issues/282#issuecomment-595163081 const dismissedWarnings: Reducer = handleActions( @@ -31,13 +33,9 @@ const dismissedWarnings: Reducer = handleActions( action: DismissFormWarning ): DismissedWarningState => { const { type } = action.payload - const stepId = action.payload.stepId return { ...state, - form: { - ...state.form, - [stepId]: [...(state.form[stepId] || []), type], - }, + form: state.form ? [...state.form, type] : [type], } }, DISMISS_TIMELINE_WARNING: ( @@ -45,34 +43,9 @@ const dismissedWarnings: Reducer = handleActions( action: DismissTimelineWarning ): DismissedWarningState => { const { type } = action.payload - const stepId = action.payload.stepId return { ...state, - timeline: { - ...state.timeline, - [stepId]: [...(state.timeline[stepId] || []), type], - }, - } - }, - DELETE_STEP: ( - state: DismissedWarningState, - action: DeleteStepAction - ): DismissedWarningState => { - // remove key for deleted step - const stepId = action.payload - return { - form: omit(state.form, stepId), - timeline: omit(state.timeline, stepId), - } - }, - DELETE_MULTIPLE_STEPS: ( - state: DismissedWarningState, - action: DeleteMultipleStepsAction - ): DismissedWarningState => { - const stepIds = action.payload - return { - form: omit(state.form, stepIds), - timeline: omit(state.timeline, stepIds), + timeline: state.timeline ? [...state.timeline, type] : [type], } }, LOAD_FILE: ( @@ -80,17 +53,10 @@ const dismissedWarnings: Reducer = handleActions( action: LoadFileAction ): DismissedWarningState => getPDMetadata(action.payload.file).dismissedWarnings, - CANCEL_STEP_FORM: ( - state: DismissedWarningState, - action: CancelStepFormAction - ): DismissedWarningState => ({ - form: omit(state.form, PRESAVED_STEP_ID), - timeline: omit(state.timeline, PRESAVED_STEP_ID), - }), }, { - form: {}, - timeline: {}, + form: [], + timeline: [], } ) export const _allReducers = { diff --git a/protocol-designer/src/dismiss/selectors.ts b/protocol-designer/src/dismiss/selectors.ts index 5a6964aea0a..289ed5e75ca 100644 --- a/protocol-designer/src/dismiss/selectors.ts +++ b/protocol-designer/src/dismiss/selectors.ts @@ -1,40 +1,26 @@ -import { createSelector } from 'reselect' import mapValues from 'lodash/mapValues' +import { createSelector } from 'reselect' import { selectors as stepFormSelectors } from '../step-forms' -import { getSelectedStepId } from '../ui/steps/selectors' -import { PRESAVED_STEP_ID } from '../steplist/types' import type { FormWarning } from '../steplist' import type { BaseState, Selector } from '../types' -import type { - RootState, - DismissedWarningsAllSteps, - WarningType, -} from './reducers' +import type { RootState, WarningType } from './reducers' + export const rootSelector = (state: BaseState): RootState => state.dismiss export const getAllDismissedWarnings: Selector = createSelector( rootSelector, s => s.dismissedWarnings ) -export const getDismissedFormWarningTypesPerStep: Selector = createSelector( - getAllDismissedWarnings, - all => all.form -) -export const getDismissedTimelineWarningTypes: Selector = createSelector( - getAllDismissedWarnings, - all => all.timeline -) +export const getDismissedFormWarningTypesPerStep: Selector< + WarningType[] +> = createSelector(getAllDismissedWarnings, all => all.form) +export const getDismissedTimelineWarningTypes: Selector< + WarningType[] +> = createSelector(getAllDismissedWarnings, all => all.timeline) export const getDismissedFormWarningTypesForSelectedStep: Selector< WarningType[] > = createSelector( getDismissedFormWarningTypesPerStep, - getSelectedStepId, - (dismissedWarnings, stepId) => { - if (stepId == null) { - return dismissedWarnings[PRESAVED_STEP_ID] || [] - } - - return dismissedWarnings[stepId] || [] - } + dismissedWarnings => dismissedWarnings ) /** Non-dismissed form-level warnings for selected step */ @@ -44,9 +30,8 @@ export const getFormWarningsForSelectedStep: Selector< stepFormSelectors.getFormLevelWarningsForUnsavedForm, getDismissedFormWarningTypesForSelectedStep, (warnings, dismissedWarnings) => { - const dismissedTypesForStep = dismissedWarnings const formWarnings = warnings.filter( - w => !dismissedTypesForStep.includes(w.type) + w => !dismissedWarnings.includes(w.type) ) return formWarnings } @@ -61,7 +46,7 @@ export const getHasFormLevelWarningsPerStep: Selector< warningsPerStep, (warnings: FormWarning, stepId: string) => (warningsPerStep[stepId] || []).filter( - w => !(dismissedPerStep[stepId] || []).includes(w.type) + w => !dismissedPerStep.includes(w.type) ).length > 0 ) ) diff --git a/protocol-designer/src/file-data/__fixtures__/createFile/commonFields.ts b/protocol-designer/src/file-data/__fixtures__/createFile/commonFields.ts index 3671654514a..14ae24e1001 100644 --- a/protocol-designer/src/file-data/__fixtures__/createFile/commonFields.ts +++ b/protocol-designer/src/file-data/__fixtures__/createFile/commonFields.ts @@ -30,8 +30,8 @@ export const fileMetadata: FileMetadataFields = { created: 1582667312515, } export const dismissedWarnings: DismissedWarningState = { - form: {}, - timeline: {}, + form: [], + timeline: [], } export const ingredients: IngredientsState = {} export const ingredLocations: LabwareLiquidState = {} diff --git a/protocol-designer/src/load-file/migration/8_1_0.ts b/protocol-designer/src/load-file/migration/8_1_0.ts index 1733316120d..cd92de7c004 100644 --- a/protocol-designer/src/load-file/migration/8_1_0.ts +++ b/protocol-designer/src/load-file/migration/8_1_0.ts @@ -5,6 +5,7 @@ import type { ProtocolFile, PipetteName, } from '@opentrons/shared-data' +import type { LegacyDismissedWarningState } from '../../dismiss/reducers' import type { DesignerApplicationData } from './utils/getLoadLiquidCommands' export interface DesignerApplicationDataV8 { @@ -24,6 +25,7 @@ export interface DesignerApplicationDataV8 { savedStepForms: Record orderedStepIds: string[] pipetteTiprackAssignments: Record + dismissedWarnings: LegacyDismissedWarningState } export const migrateFile = ( @@ -46,6 +48,8 @@ export const migrateFile = ( {} ) + const dismissedWarnings = designerApplication.data?.dismissedWarnings + const pipetteTiprackAssignments = designerApplication.data?.pipetteTiprackAssignments @@ -131,6 +135,19 @@ export const migrateFile = ( {} ) + const newDismissedWarningsForm = + dismissedWarnings.form != null + ? Object.values(dismissedWarnings.form).flatMap( + formType => formType as string[] + ) + : [] + const newDismissedWarningsTimeline = + dismissedWarnings.timeline != null + ? Object.values(dismissedWarnings.timeline).flatMap( + timelineType => timelineType as string[] + ) + : [] + return { ...appData, designerApplication: { @@ -142,6 +159,10 @@ export const migrateFile = ( ...pipettingSavedStepsWithAdditionalFields, }, pipetteTiprackAssignments: newTiprackAssignments, + dismissedWarnings: { + form: newDismissedWarningsForm, + timeline: newDismissedWarningsTimeline, + }, }, }, } diff --git a/protocol-designer/src/load-file/migration/utils/getLoadLiquidCommands.ts b/protocol-designer/src/load-file/migration/utils/getLoadLiquidCommands.ts index 9a48688a1d0..2be01b2e4b9 100644 --- a/protocol-designer/src/load-file/migration/utils/getLoadLiquidCommands.ts +++ b/protocol-designer/src/load-file/migration/utils/getLoadLiquidCommands.ts @@ -1,6 +1,7 @@ import reduce from 'lodash/reduce' import { uuid } from '../../../utils' import type { LoadLiquidCreateCommand } from '@opentrons/shared-data/protocol/types/schemaV6/command/setup' +import type { DismissedWarningState } from '../../../dismiss/reducers' export interface DesignerApplicationData { ingredients: Record< @@ -19,6 +20,7 @@ export interface DesignerApplicationData { savedStepForms: Record orderedStepIds: string[] pipetteTiprackAssignments: Record + dismissedWarnings: DismissedWarningState } export const getLoadLiquidCommands = ( diff --git a/protocol-designer/src/top-selectors/timelineWarnings/index.ts b/protocol-designer/src/top-selectors/timelineWarnings/index.ts index 2a8e3252af9..262e4fc0eab 100644 --- a/protocol-designer/src/top-selectors/timelineWarnings/index.ts +++ b/protocol-designer/src/top-selectors/timelineWarnings/index.ts @@ -14,7 +14,7 @@ export const getTimelineWarningsForSelectedStep: Selector< (dismissedWarningTypes, warningsPerStep, stepId) => { if (stepId == null) return [] return (warningsPerStep[stepId] || []).filter( - warning => !(dismissedWarningTypes[stepId] || []).includes(warning.type) + warning => !dismissedWarningTypes.includes(warning.type) ) } ) @@ -28,12 +28,9 @@ export const getHasTimelineWarningsPerStep: Selector = creat const warningTypesForStep = (warningsPerStep[stepId] || []).map( w => w.type ) - const dismissedWarningTypesForStep = new Set( - dismissedWarningTypes[stepId] || [] - ) const hasUndismissedWarnings = warningTypesForStep.filter( - warningType => !dismissedWarningTypesForStep.has(warningType) + warningType => !dismissedWarningTypes.includes(warningType) ).length > 0 return { ...stepAcc, [stepId]: hasUndismissedWarnings } }, {}) From ba28337d097849690e53505d5dd0eb910d4aaf67 Mon Sep 17 00:00:00 2001 From: Nick Diehl <47604184+ncdiehl11@users.noreply.github.com> Date: Wed, 22 May 2024 14:21:17 -0400 Subject: [PATCH 11/17] fix(protocol-designer): format export modal mesasge for repeat modules and 96-channel (#15249) Closes RQA-2762 Consolidate modules of the same type into a single plural noun rather than 'and'-joined repeated modules. Remove mount side for 96-channel pipette. --- .../components/FileSidebar/FileSidebar.tsx | 49 +++++++++++++++-- .../__tests__/FileSidebar.test.tsx | 54 +++++++++++++++++++ 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/protocol-designer/src/components/FileSidebar/FileSidebar.tsx b/protocol-designer/src/components/FileSidebar/FileSidebar.tsx index 11b8d21053d..b083a17230f 100644 --- a/protocol-designer/src/components/FileSidebar/FileSidebar.tsx +++ b/protocol-designer/src/components/FileSidebar/FileSidebar.tsx @@ -127,12 +127,53 @@ function getWarningContent({ } const pipettesDetails = pipettesWithoutStep - .map(pipette => `${pipette.mount} ${pipette.spec.displayName}`) + .map(pipette => + pipette.spec.channels === 96 + ? `${pipette.spec.displayName} pipette` + : `${pipette.mount} ${pipette.spec.displayName} pipette` + ) .join(' and ') - const modulesDetails = modulesWithoutStep - .map(moduleOnDeck => t(`modules:module_long_names.${moduleOnDeck.type}`)) - .join(' and ') + const unusedModuleCounts = modulesWithoutStep.reduce<{ + [key: string]: number + }>((acc, mod) => { + if (!(mod.type in acc)) { + return { ...acc, [mod.type]: 1 } + } else { + return { ...acc, [mod.type]: acc[mod.type] + 1 } + } + }, {}) + + const modulesDetails = Object.keys(unusedModuleCounts) + // sorting by module count + .sort((k1, k2) => { + if (unusedModuleCounts[k1] < unusedModuleCounts[k2]) { + return 1 + } else if (unusedModuleCounts[k1] > unusedModuleCounts[k2]) { + return -1 + } else { + return 0 + } + }) + .map(modType => + unusedModuleCounts[modType] === 1 + ? t(`modules:module_long_names.${modType}`) + : `${t(`modules:module_long_names.${modType}`)}s` + ) + // transform list of modules with counts to string + .reduce((acc, modName, index, arr) => { + if (arr.length > 2) { + if (index === arr.length - 1) { + return `${acc} and ${modName}` + } else { + return `${acc}${modName}, ` + } + } else if (arr.length === 2) { + return index === 0 ? `${modName} and ` : `${acc}${modName}` + } else { + return modName + } + }, '') if (pipettesWithoutStep.length && modulesWithoutStep.length) { return { diff --git a/protocol-designer/src/components/FileSidebar/__tests__/FileSidebar.test.tsx b/protocol-designer/src/components/FileSidebar/__tests__/FileSidebar.test.tsx index 8d0d38ec6cf..11bda8345c6 100644 --- a/protocol-designer/src/components/FileSidebar/__tests__/FileSidebar.test.tsx +++ b/protocol-designer/src/components/FileSidebar/__tests__/FileSidebar.test.tsx @@ -230,4 +230,58 @@ describe('FileSidebar', () => { 'One or more modules specified in your protocol in Slot(s) A1,B1 are not currently used in any step. In order to run this protocol you will need to power up and connect the modules to your robot.' ) }) + it('renders the formatted unused pipettes and modules warning sorted by count', () => { + vi.mocked(getInitialDeckSetup).mockReturnValue({ + modules: { + moduleId1: { + slot: 'A1', + moduleState: {} as any, + id: 'moduleId', + type: 'thermocyclerModuleType', + model: 'thermocyclerModuleV2', + }, + moduleId2: { + slot: 'C3', + moduleState: {} as any, + id: 'moduleId1', + type: 'temperatureModuleType', + model: 'temperatureModuleV2', + }, + moduleId3: { + slot: 'D3', + moduleState: {} as any, + id: 'moduleId2', + type: 'temperatureModuleType', + model: 'temperatureModuleV2', + }, + moduleId4: { + slot: 'C1', + moduleState: {} as any, + id: 'moduleId3', + type: 'heaterShakerModuleType', + model: 'heaterShakerModuleV1', + }, + }, + pipettes: { + pipetteId: { + mount: 'left', + name: 'p1000_96', + id: 'pipetteId', + tiprackLabwareDef: [fixtureTiprack300ul as LabwareDefinition2], + tiprackDefURI: ['mockDefUri'], + spec: { + displayName: 'mock display name', + channels: 96, + } as any, + }, + }, + additionalEquipmentOnDeck: {}, + labware: {}, + }) + render() + fireEvent.click(screen.getByRole('button', { name: 'Export' })) + screen.getByText( + 'The mock display name pipette and Temperature modules, Thermocycler module, and Heater-Shaker module in your protocol are not currently used in any step. In order to run this protocol you will need to attach this pipette as well as power up and connect the module to your robot.' + ) + }) }) From 51139bc24b3013246b4f7087baeafdd61e10d978 Mon Sep 17 00:00:00 2001 From: Jethary Rader <66035149+jerader@users.noreply.github.com> Date: Wed, 22 May 2024 14:29:54 -0400 Subject: [PATCH 12/17] fix(protocol-designer): missing labware modal logic (#15248) closes RQA-2769 --- protocol-designer/src/ui/modules/selectors.ts | 8 ++++++ .../addAndSelectStepWithHints.test.ts | 28 +++++++++++++++++-- .../src/ui/steps/actions/thunks/index.ts | 19 +++++++++---- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/protocol-designer/src/ui/modules/selectors.ts b/protocol-designer/src/ui/modules/selectors.ts index ea8f3ba5433..03e0886833f 100644 --- a/protocol-designer/src/ui/modules/selectors.ts +++ b/protocol-designer/src/ui/modules/selectors.ts @@ -117,6 +117,14 @@ export const getMagnetModuleHasLabware: Selector = createSelector( } ) +/** Returns boolean if heater-shaker module has labware */ +export const getHeaterShakerModuleHasLabware: Selector = createSelector( + getInitialDeckSetup, + initialDeckSetup => { + return getModuleHasLabware(initialDeckSetup, HEATERSHAKER_MODULE_TYPE) + } +) + /** Returns all moduleIds and if they have labware for MoaM */ export const getTemperatureModulesHaveLabware: Selector< ModuleAndLabware[] diff --git a/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.ts b/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.ts index 56046da6a98..fa749bfa4e2 100644 --- a/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.ts +++ b/protocol-designer/src/ui/steps/actions/__tests__/addAndSelectStepWithHints.test.ts @@ -26,6 +26,9 @@ beforeEach(() => { false ) vi.mocked(uiModuleSelectors.getTemperatureModuleIds).mockReturnValue(null) + vi.mocked(uiModuleSelectors.getHeaterShakerModuleHasLabware).mockReturnValue( + false + ) vi.mocked(uiModuleSelectors.getSingleThermocyclerModuleId).mockReturnValue( null ) @@ -92,6 +95,7 @@ describe('addAndSelectStepWithHints', () => { getSingleTemperatureModuleId: null, getSingleThermocyclerModuleId: null, getTemperatureModuleIds: [], + getHeaterShakerModuleHasLabware: false, }, }, { @@ -105,12 +109,13 @@ describe('addAndSelectStepWithHints', () => { getThermocyclerModuleHasLabware: false, getSingleTemperatureModuleId: 'something', getSingleThermocyclerModuleId: null, - getTemperatureModuleIds: ['mockId'], + getTemperatureModuleIds: [], + getHeaterShakerModuleHasLabware: false, }, }, { - testName: 'temperature step, when thermocycler has no labware', - stepType: 'temperature' as StepType, + testName: 'thermocycler step, when thermocycler has no labware', + stepType: 'thermocycler' as StepType, selectorValues: { getMagnetModuleHasLabware: false, getTemperatureModulesHaveLabware: [], @@ -118,6 +123,20 @@ describe('addAndSelectStepWithHints', () => { getSingleTemperatureModuleId: null, getSingleThermocyclerModuleId: 'something', getTemperatureModuleIds: [], + getHeaterShakerModuleHasLabware: false, + }, + }, + { + testName: 'heaterShaker step, when heaterShaker has no labware', + stepType: 'heaterShaker' as StepType, + selectorValues: { + getMagnetModuleHasLabware: false, + getTemperatureModulesHaveLabware: [], + getThermocyclerModuleHasLabware: false, + getSingleTemperatureModuleId: null, + getSingleThermocyclerModuleId: 'something', + getTemperatureModuleIds: [], + getHeaterShakerModuleHasLabware: false, }, }, ].forEach(({ testName, stepType, selectorValues }) => { @@ -128,6 +147,9 @@ describe('addAndSelectStepWithHints', () => { vi.mocked( uiModuleSelectors.getTemperatureModulesHaveLabware ).mockReturnValue(selectorValues.getTemperatureModulesHaveLabware) + vi.mocked( + uiModuleSelectors.getHeaterShakerModuleHasLabware + ).mockReturnValue(selectorValues.getHeaterShakerModuleHasLabware) vi.mocked( uiModuleSelectors.getThermocyclerModuleHasLabware ).mockReturnValue(selectorValues.getThermocyclerModuleHasLabware) diff --git a/protocol-designer/src/ui/steps/actions/thunks/index.ts b/protocol-designer/src/ui/steps/actions/thunks/index.ts index b59780e949c..707b5e0713d 100644 --- a/protocol-designer/src/ui/steps/actions/thunks/index.ts +++ b/protocol-designer/src/ui/steps/actions/thunks/index.ts @@ -49,9 +49,10 @@ export const addAndSelectStepWithHints: (arg: { const temperatureModuleOnDeck = uiModuleSelectors.getTemperatureModuleIds( state ) - const thermocyclerModuleOnDeck = uiModuleSelectors.getSingleThermocyclerModuleId( + const heaterShakerModuleHasLabware = uiModuleSelectors.getHeaterShakerModuleHasLabware( state ) + const tempHasNoLabware = temperatureModulesHaveLabware.some( module => !module.hasLabware ) @@ -59,19 +60,25 @@ export const addAndSelectStepWithHints: (arg: { const stepNeedsLiquid = ['mix', 'moveLiquid'].includes(payload.stepType) const stepMagnetNeedsLabware = ['magnet'].includes(payload.stepType) const stepTemperatureNeedsLabware = ['temperature'].includes(payload.stepType) + const stepThermocyclerNeedsLabware = ['thermocycler'].includes( + payload.stepType + ) + const stepHeaterShakerNeedsLabware = ['heaterShaker'].includes( + payload.stepType + ) + const stepModuleMissingLabware = (stepMagnetNeedsLabware && !magnetModuleHasLabware) || - (stepTemperatureNeedsLabware && - thermocyclerModuleOnDeck && - !thermocyclerModuleHasLabware) || - (temperatureModuleOnDeck?.length === 1 && tempHasNoLabware) + (stepThermocyclerNeedsLabware && !thermocyclerModuleHasLabware) || + (temperatureModuleOnDeck?.length === 0 && stepTemperatureNeedsLabware) || + (stepHeaterShakerNeedsLabware && !heaterShakerModuleHasLabware) if (stepNeedsLiquid && !deckHasLiquid) { dispatch(tutorialActions.addHint('add_liquids_and_labware')) } if (stepModuleMissingLabware) { dispatch(tutorialActions.addHint('module_without_labware')) - } else if (temperatureModuleOnDeck && tempHasNoLabware) { + } else if (temperatureModuleOnDeck != null && tempHasNoLabware) { dispatch(tutorialActions.addHint('multiple_modules_without_labware')) } } From ccc944efb238a0f9a5d32f04aeeb308018993c97 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Wed, 22 May 2024 14:44:32 -0400 Subject: [PATCH 13/17] refactor(api): conditionally import usb dep (#15247) I think the intent was always that AsyncByonoy and thus pyusb would only be imported when actually building an AsyncByonoy instance, but at some point it got imported globally for a typecheck. Make that typecheck typecheck-only with a different name, and the problem goes away. Other side of #15206 --- api/src/opentrons/drivers/absorbance_reader/driver.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/api/src/opentrons/drivers/absorbance_reader/driver.py b/api/src/opentrons/drivers/absorbance_reader/driver.py index 538bc83d70d..8777f8cbbf7 100644 --- a/api/src/opentrons/drivers/absorbance_reader/driver.py +++ b/api/src/opentrons/drivers/absorbance_reader/driver.py @@ -1,12 +1,14 @@ from __future__ import annotations import asyncio -from typing import Dict, Optional, List +from typing import Dict, Optional, List, TYPE_CHECKING from opentrons.drivers.types import AbsorbanceReaderLidStatus from opentrons.drivers.absorbance_reader.abstract import AbstractAbsorbanceReaderDriver from opentrons.drivers.rpi_drivers.types import USBPort -from .async_byonoy import AsyncByonoy + +if TYPE_CHECKING: + from .async_byonoy import AsyncByonoy as AsyncByonoyType class AbsorbanceReaderDriver(AbstractAbsorbanceReaderDriver): @@ -23,7 +25,7 @@ async def create( connection = await AsyncByonoy.create(port=port, usb_port=usb_port, loop=loop) return cls(connection=connection) - def __init__(self, connection: AsyncByonoy) -> None: + def __init__(self, connection: AsyncByonoyType) -> None: self._connection = connection async def get_device_info(self) -> Dict[str, str]: From 4ee432c06866f672a8d9d032942b63c3240fbf1e Mon Sep 17 00:00:00 2001 From: Caila Marashaj <98041399+caila-marashaj@users.noreply.github.com> Date: Wed, 22 May 2024 15:30:39 -0400 Subject: [PATCH 14/17] feat(hardware): add hepa uv to tool detector (#15001) --- .../backends/test_ot3_subsystem_manager.py | 8 +-- .../hardware_control/tools/detector.py | 58 +++++++++++++++---- .../hardware_control/tools/types.py | 18 ++++-- 3 files changed, 64 insertions(+), 20 deletions(-) diff --git a/api/tests/opentrons/hardware_control/backends/test_ot3_subsystem_manager.py b/api/tests/opentrons/hardware_control/backends/test_ot3_subsystem_manager.py index 86c2d70459b..a7690fde28a 100644 --- a/api/tests/opentrons/hardware_control/backends/test_ot3_subsystem_manager.py +++ b/api/tests/opentrons/hardware_control/backends/test_ot3_subsystem_manager.py @@ -227,10 +227,10 @@ def _pipette_info_from_network( else default_name ) return tools.types.PipetteInformation( - pipette_name, - pipette_name.value, - pipette_name.name, - f"dummyserial{pipette_name.name}", + name=pipette_name, + name_int=pipette_name.value, + model=pipette_name.name, + serial=f"dummyserial{pipette_name.name}", ) def _auto_tool_summary( diff --git a/hardware/opentrons_hardware/hardware_control/tools/detector.py b/hardware/opentrons_hardware/hardware_control/tools/detector.py index fe5f04032ed..7483886c52f 100644 --- a/hardware/opentrons_hardware/hardware_control/tools/detector.py +++ b/hardware/opentrons_hardware/hardware_control/tools/detector.py @@ -16,6 +16,7 @@ PipetteInformation, ToolSummary, GripperInformation, + HepaUVInformation, ) from opentrons_hardware.hardware_control.tools.types import ToolDetectionResult @@ -58,7 +59,7 @@ def _decode_or_default(orig: bytes) -> str: async def _await_responses( callback: WaitableCallback, for_nodes: Set[NodeId], - response_queue: "asyncio.Queue[Tuple[NodeId, Union[PipetteInformation, GripperInformation]]]", + response_queue: "asyncio.Queue[Tuple[NodeId, Union[PipetteInformation, GripperInformation, HepaUVInformation]]]", ) -> None: """Wait for pipette or gripper information and send back through a queue.""" seen: Set[NodeId] = set() @@ -77,12 +78,34 @@ async def _await_responses( ) seen.add(node) break + elif isinstance(response, message_definitions.HepaUVInfoResponse): + node = await _handle_hepa_uv_info( + response_queue, response, arbitration_id + ) elif isinstance(response, message_definitions.ErrorMessage): log.error(f"Received error message {str(response)}") +async def _handle_hepa_uv_info( + response_queue: "asyncio.Queue[Tuple[NodeId, Union[PipetteInformation, GripperInformation, HepaUVInformation]]]", + response: message_definitions.HepaUVInfoResponse, + arbitration_id: ArbitrationId, +) -> NodeId: + node = NodeId(arbitration_id.parts.originating_node_id) + await response_queue.put( + ( + node, + HepaUVInformation( + model=model_versionstring_from_int(response.payload.model.value), + serial=_decode_or_default(response.payload.serial.value), + ), + ) + ) + return node + + async def _handle_gripper_info( - response_queue: "asyncio.Queue[Tuple[NodeId, Union[PipetteInformation, GripperInformation]]]", + response_queue: "asyncio.Queue[Tuple[NodeId, Union[PipetteInformation, GripperInformation, HepaUVInformation]]]", response: message_definitions.GripperInfoResponse, arbitration_id: ArbitrationId, ) -> NodeId: @@ -100,7 +123,7 @@ async def _handle_gripper_info( async def _handle_pipette_info( - response_queue: "asyncio.Queue[Tuple[NodeId, Union[PipetteInformation, GripperInformation]]]", + response_queue: "asyncio.Queue[Tuple[NodeId, Union[PipetteInformation, GripperInformation, HepaUVInformation]]]", response: message_definitions.PipetteInfoResponse, arbitration_id: ArbitrationId, ) -> NodeId: @@ -139,7 +162,9 @@ def _should_query(attach_response: ToolType) -> bool: IntermediateResolution = Tuple[ - Dict[NodeId, PipetteInformation], Dict[NodeId, GripperInformation] + Dict[NodeId, PipetteInformation], + Dict[NodeId, GripperInformation], + Dict[NodeId, HepaUVInformation], ] @@ -153,7 +178,7 @@ async def _do_tool_resolve( node_id=NodeId.broadcast, message=message_definitions.InstrumentInfoRequest(), ) - incoming_queue: "asyncio.Queue[Tuple[NodeId, Union[PipetteInformation, GripperInformation]]]" = ( + incoming_queue: "asyncio.Queue[Tuple[NodeId, Union[PipetteInformation, GripperInformation, HepaUVInformation]]]" = ( asyncio.Queue() ) try: @@ -166,14 +191,17 @@ async def _do_tool_resolve( pipettes: Dict[NodeId, PipetteInformation] = {} gripper: Dict[NodeId, GripperInformation] = {} + hepa_uv: Dict[NodeId, HepaUVInformation] = {} while not incoming_queue.empty(): node, info = incoming_queue.get_nowait() if isinstance(info, PipetteInformation): pipettes[node] = info - else: + elif isinstance(info, GripperInformation): gripper[node] = info + elif isinstance(info, HepaUVInformation): + hepa_uv[node] = info - return pipettes, gripper + return pipettes, gripper, hepa_uv async def _resolve_with_stimulus_retries( @@ -187,15 +215,23 @@ async def _resolve_with_stimulus_retries( should_respond ) expected_gripper = {NodeId.gripper}.intersection(should_respond) + expected_hepa_uv = {NodeId.hepa_uv}.intersection(should_respond) while True: - pipettes, gripper = await _do_tool_resolve( + pipettes, gripper, hepa_uv = await _do_tool_resolve( messenger, wc, should_respond, attempt_timeout_sec ) - output_queue.put_nowait((pipettes, gripper)) + output_queue.put_nowait((pipettes, gripper, hepa_uv)) seen_pipettes = set([k.application_for() for k in pipettes.keys()]) seen_gripper = set([k.application_for() for k in gripper.keys()]) - if seen_pipettes == expected_pipettes and seen_gripper == expected_gripper: + seen_hepa_uv = set([k.application_for() for k in hepa_uv.keys()]) + if all( + [ + seen_pipettes == expected_pipettes, + seen_gripper == expected_gripper, + seen_hepa_uv == expected_hepa_uv, + ] + ): return @@ -222,7 +258,7 @@ async def _resolve_tool_types( except asyncio.TimeoutError: log.warning("No response from expected tool") - last_element: IntermediateResolution = ({}, {}) + last_element: IntermediateResolution = ({}, {}, {}) try: while True: last_element = resolve_queue.get_nowait() diff --git a/hardware/opentrons_hardware/hardware_control/tools/types.py b/hardware/opentrons_hardware/hardware_control/tools/types.py index 1d9d4286931..48439b5a444 100644 --- a/hardware/opentrons_hardware/hardware_control/tools/types.py +++ b/hardware/opentrons_hardware/hardware_control/tools/types.py @@ -15,21 +15,29 @@ class ToolDetectionResult: @dataclass(frozen=True) -class GripperInformation: - """Model the information you can retrieve from a gripper.""" +class BaseToolInformation: + """Model the base information you can retrieve from any tool.""" model: str serial: str @dataclass(frozen=True) -class PipetteInformation: +class GripperInformation(BaseToolInformation): + """Model the information you can retrieve from a gripper.""" + + +@dataclass(frozen=True) +class HepaUVInformation(BaseToolInformation): + """Model the information you can retrieve from a hepa/uv device.""" + + +@dataclass(frozen=True) +class PipetteInformation(BaseToolInformation): """Model the information you can retrieve from a pipette.""" name: PipetteName name_int: int - model: str - serial: str @dataclass(frozen=True) From d5eb9c74b1c97e27b753f1cde121cd78acd2566a Mon Sep 17 00:00:00 2001 From: Ryan Howard Date: Thu, 23 May 2024 11:30:09 -0400 Subject: [PATCH 15/17] chore(hardware-testing): Run the hardware testing github actions on more PR's (#15233) A change that broke hardware testing snuck into edge because the github actions didn't trigger on PR's that that didn't touch the hardware testing folder this makes the hardware-testing workflow run on subdirectories that the hardware-testing subdirectory depends on so that we avoid this in the future. # Overview # Test Plan # Changelog # Review requests # Risk assessment --- .github/workflows/hardware-testing.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/hardware-testing.yaml b/.github/workflows/hardware-testing.yaml index 6977194ca2a..bb738b13e4b 100644 --- a/.github/workflows/hardware-testing.yaml +++ b/.github/workflows/hardware-testing.yaml @@ -9,6 +9,9 @@ on: paths: - 'Makefile' - 'hardware-testing/**' + - 'api/**' + - 'shared-data/**' + - 'hardware/**' - '.github/workflows/hardware-testing.yaml' - '.github/actions/python/**' branches: From 9a877eca253ba74bf260cbf369263bf334f7249b Mon Sep 17 00:00:00 2001 From: Ryan Howard Date: Thu, 23 May 2024 15:54:51 -0400 Subject: [PATCH 16/17] chore(api): clean up lld hardware layer (#15200) # Overview This PR removes some uneeded arguments and fixes a few #FIXME that were hanging around # Test Plan # Changelog # Review requests # Risk assessment --- api/src/opentrons/config/defaults_ot3.py | 2 -- api/src/opentrons/config/types.py | 1 - .../backends/ot3controller.py | 9 +++++++-- api/src/opentrons/hardware_control/ot3api.py | 19 ++++++++++++++----- api/tests/opentrons/config/ot3_settings.py | 1 - .../backends/test_ot3_controller.py | 1 - .../hardware_control/test_ot3_api.py | 15 ++++++++++----- .../hardware_testing/gravimetric/config.py | 12 ------------ .../hardware_testing/liquid_sense/execute.py | 1 - .../pipette_assembly_qc_ot3/__main__.py | 1 - 10 files changed, 31 insertions(+), 31 deletions(-) diff --git a/api/src/opentrons/config/defaults_ot3.py b/api/src/opentrons/config/defaults_ot3.py index 0b2499feaab..9dccb1566d8 100644 --- a/api/src/opentrons/config/defaults_ot3.py +++ b/api/src/opentrons/config/defaults_ot3.py @@ -25,7 +25,6 @@ DEFAULT_LIQUID_PROBE_SETTINGS: Final[LiquidProbeSettings] = LiquidProbeSettings( starting_mount_height=100, max_z_distance=40, - min_z_distance=5, mount_speed=10, plunger_speed=5, sensor_threshold_pascals=40, @@ -337,7 +336,6 @@ def _build_default_liquid_probe( "starting_mount_height", default.starting_mount_height ), max_z_distance=from_conf.get("max_z_distance", default.max_z_distance), - min_z_distance=from_conf.get("min_z_distance", default.min_z_distance), mount_speed=from_conf.get("mount_speed", default.mount_speed), plunger_speed=from_conf.get("plunger_speed", default.plunger_speed), sensor_threshold_pascals=from_conf.get( diff --git a/api/src/opentrons/config/types.py b/api/src/opentrons/config/types.py index f13d5a5e6e3..476c3181dc2 100644 --- a/api/src/opentrons/config/types.py +++ b/api/src/opentrons/config/types.py @@ -130,7 +130,6 @@ class OutputOptions(int, Enum): class LiquidProbeSettings: starting_mount_height: float max_z_distance: float - min_z_distance: float mount_speed: float plunger_speed: float sensor_threshold_pascals: float diff --git a/api/src/opentrons/hardware_control/backends/ot3controller.py b/api/src/opentrons/hardware_control/backends/ot3controller.py index 1b90e086dca..62d44ae7aea 100644 --- a/api/src/opentrons/hardware_control/backends/ot3controller.py +++ b/api/src/opentrons/hardware_control/backends/ot3controller.py @@ -194,6 +194,7 @@ LiquidNotFoundError, CommunicationError, PythonException, + UnsupportedHardwareCommand, ) from .subsystem_manager import SubsystemManager @@ -1367,12 +1368,16 @@ async def liquid_probe( probe: InstrumentProbeType = InstrumentProbeType.PRIMARY, ) -> float: if output_option == OutputOptions.sync_buffer_to_csv: - assert ( + if ( self._subsystem_manager.device_info[ SubSystem.of_mount(mount) ].revision.tertiary == "1" - ) + ): + raise UnsupportedHardwareCommand( + "Liquid Probe not supported on this pipette firmware" + ) + head_node = axis_to_node(Axis.by_mount(mount)) tool = sensor_node_for_pipette(OT3Mount(mount.value)) csv_output = bool(output_option.value & OutputOptions.stream_to_csv.value) diff --git a/api/src/opentrons/hardware_control/ot3api.py b/api/src/opentrons/hardware_control/ot3api.py index c50dc08aebb..8e4698f629e 100644 --- a/api/src/opentrons/hardware_control/ot3api.py +++ b/api/src/opentrons/hardware_control/ot3api.py @@ -2580,13 +2580,22 @@ async def liquid_probe( if probe_settings.aspirate_while_sensing: await self._move_to_plunger_bottom(mount, rate=1.0) else: - # TODO: shorten this distance by only moving just far enough - # to account for the specified "max-z-distance" + # find the ideal travel distance by multiplying the plunger speed + # by the time it will take to complete the z move. + ideal_travel = probe_settings.plunger_speed * ( + probe_settings.max_z_distance / probe_settings.mount_speed + ) + + # TODO limit the z distance to the max allowed by the plunger travel at this speed. + # or here is probably the ideal place to implement multi-probe + assert ( + instrument.plunger_positions.bottom - ideal_travel + >= instrument.plunger_positions.top + ) + target_point = instrument.plunger_positions.bottom - ideal_travel target_pos = target_position_from_plunger( - checked_mount, instrument.plunger_positions.top, self._current_position + checked_mount, target_point, self._current_position ) - # FIXME: this should really be the slower "aspirate" speed, - # but this is still in testing phase so let's bias towards speed max_speeds = self.config.motion_settings.default_max_speed speed = max_speeds[self.gantry_load][OT3AxisKind.P] await self._move(target_pos, speed=speed, acquire_lock=True) diff --git a/api/tests/opentrons/config/ot3_settings.py b/api/tests/opentrons/config/ot3_settings.py index 3cfa9b7c34c..eb15cb8efe3 100644 --- a/api/tests/opentrons/config/ot3_settings.py +++ b/api/tests/opentrons/config/ot3_settings.py @@ -120,7 +120,6 @@ "liquid_sense": { "starting_mount_height": 80, "max_z_distance": 20, - "min_z_distance": 3, "mount_speed": 10, "plunger_speed": 10, "sensor_threshold_pascals": 17, diff --git a/api/tests/opentrons/hardware_control/backends/test_ot3_controller.py b/api/tests/opentrons/hardware_control/backends/test_ot3_controller.py index fd2d0469954..b57a4b2ab4b 100644 --- a/api/tests/opentrons/hardware_control/backends/test_ot3_controller.py +++ b/api/tests/opentrons/hardware_control/backends/test_ot3_controller.py @@ -178,7 +178,6 @@ def fake_liquid_settings() -> LiquidProbeSettings: return LiquidProbeSettings( starting_mount_height=100, max_z_distance=15, - min_z_distance=5, mount_speed=40, plunger_speed=10, sensor_threshold_pascals=15, diff --git a/api/tests/opentrons/hardware_control/test_ot3_api.py b/api/tests/opentrons/hardware_control/test_ot3_api.py index 7ab0a2f1c00..7a57978bc14 100644 --- a/api/tests/opentrons/hardware_control/test_ot3_api.py +++ b/api/tests/opentrons/hardware_control/test_ot3_api.py @@ -115,7 +115,6 @@ def fake_liquid_settings() -> LiquidProbeSettings: return LiquidProbeSettings( starting_mount_height=100, max_z_distance=15, - min_z_distance=10, mount_speed=40, plunger_speed=10, sensor_threshold_pascals=15, @@ -776,12 +775,19 @@ async def test_liquid_probe( pipette_node: Axis, mount: OT3Mount, fake_liquid_settings: LiquidProbeSettings, - mock_instrument_handlers: Tuple[MagicMock], mock_current_position_ot3: AsyncMock, - mock_ungrip: AsyncMock, mock_move_to_plunger_bottom: AsyncMock, ) -> None: - mock_ungrip.return_value = None + instr_data = AttachedPipette( + config=load_pipette_data.load_definition( + PipetteModelType("p1000"), PipetteChannelType(1), PipetteVersionType(3, 4) + ), + id="fakepip", + ) + await ot3_hardware.cache_pipette(mount, instr_data, None) + pipette = ot3_hardware.hardware_pipettes[mount.to_mount()] + assert pipette + await ot3_hardware.add_tip(mount, 100) await ot3_hardware.home() mock_move_to.return_value = None @@ -800,7 +806,6 @@ async def test_liquid_probe( fake_settings_aspirate = LiquidProbeSettings( starting_mount_height=100, max_z_distance=15, - min_z_distance=5, mount_speed=40, plunger_speed=10, sensor_threshold_pascals=15, diff --git a/hardware-testing/hardware_testing/gravimetric/config.py b/hardware-testing/hardware_testing/gravimetric/config.py index f80d87d7124..394309b43f4 100644 --- a/hardware-testing/hardware_testing/gravimetric/config.py +++ b/hardware-testing/hardware_testing/gravimetric/config.py @@ -90,7 +90,6 @@ class PhotometricConfig(VolumetricConfig): 1: { 50: { "max_z_distance": 20, - "min_z_distance": 5, "mount_speed": 11, "plunger_speed": 21, "sensor_threshold_pascals": 150, @@ -99,7 +98,6 @@ class PhotometricConfig(VolumetricConfig): 8: { 50: { "max_z_distance": 20, - "min_z_distance": 5, "mount_speed": 11, "plunger_speed": 21, "sensor_threshold_pascals": 150, @@ -110,21 +108,18 @@ class PhotometricConfig(VolumetricConfig): 1: { 50: { "max_z_distance": 20, - "min_z_distance": 5, "mount_speed": 5, "plunger_speed": 10, "sensor_threshold_pascals": 200, }, 200: { "max_z_distance": 20, - "min_z_distance": 5, "mount_speed": 5, "plunger_speed": 10, "sensor_threshold_pascals": 200, }, 1000: { "max_z_distance": 20, - "min_z_distance": 5, "mount_speed": 5, "plunger_speed": 11, "sensor_threshold_pascals": 150, @@ -133,21 +128,18 @@ class PhotometricConfig(VolumetricConfig): 8: { 50: { "max_z_distance": 20, - "min_z_distance": 5, "mount_speed": 5, "plunger_speed": 10, "sensor_threshold_pascals": 200, }, 200: { "max_z_distance": 20, - "min_z_distance": 5, "mount_speed": 5, "plunger_speed": 10, "sensor_threshold_pascals": 200, }, 1000: { "max_z_distance": 20, - "min_z_distance": 5, "mount_speed": 5, "plunger_speed": 11, "sensor_threshold_pascals": 150, @@ -156,21 +148,18 @@ class PhotometricConfig(VolumetricConfig): 96: { 50: { "max_z_distance": 20, - "min_z_distance": 5, "mount_speed": 5, "plunger_speed": 10, "sensor_threshold_pascals": 200, }, 200: { "max_z_distance": 20, - "min_z_distance": 5, "mount_speed": 5, "plunger_speed": 10, "sensor_threshold_pascals": 200, }, 1000: { "max_z_distance": 20, - "min_z_distance": 5, "mount_speed": 5, "plunger_speed": 11, "sensor_threshold_pascals": 150, @@ -189,7 +178,6 @@ def _get_liquid_probe_settings( return LiquidProbeSettings( starting_mount_height=well.top().point.z, max_z_distance=min(well.depth, lqid_cfg["max_z_distance"]), - min_z_distance=lqid_cfg["min_z_distance"], mount_speed=lqid_cfg["mount_speed"], plunger_speed=lqid_cfg["plunger_speed"], sensor_threshold_pascals=lqid_cfg["sensor_threshold_pascals"], diff --git a/hardware-testing/hardware_testing/liquid_sense/execute.py b/hardware-testing/hardware_testing/liquid_sense/execute.py index 9a61c172c8e..05be865015f 100644 --- a/hardware-testing/hardware_testing/liquid_sense/execute.py +++ b/hardware-testing/hardware_testing/liquid_sense/execute.py @@ -355,7 +355,6 @@ def _run_trial(run_args: RunArgs, tip: int, well: Well, trial: int) -> float: lps = LiquidProbeSettings( starting_mount_height=start_height, max_z_distance=z_dist, - min_z_distance=lqid_cfg["min_z_distance"], mount_speed=run_args.z_speed, plunger_speed=plunger_speed, sensor_threshold_pascals=lqid_cfg["sensor_threshold_pascals"], diff --git a/hardware-testing/hardware_testing/production_qc/pipette_assembly_qc_ot3/__main__.py b/hardware-testing/hardware_testing/production_qc/pipette_assembly_qc_ot3/__main__.py index 5e482afa6e7..fabdb101256 100644 --- a/hardware-testing/hardware_testing/production_qc/pipette_assembly_qc_ot3/__main__.py +++ b/hardware-testing/hardware_testing/production_qc/pipette_assembly_qc_ot3/__main__.py @@ -1377,7 +1377,6 @@ async def _test_liquid_probe( probe_settings = LiquidProbeSettings( starting_mount_height=start_pos.z, max_z_distance=max_z_distance_machine_coords, # FIXME: deck coords - min_z_distance=0, # FIXME: remove mount_speed=probe_cfg.mount_speed, plunger_speed=probe_cfg.plunger_speed, sensor_threshold_pascals=probe_cfg.sensor_threshold_pascals, From 39dee3aadcf2783a47d319a831ef5a6649a2c3c5 Mon Sep 17 00:00:00 2001 From: Brayan Almonte Date: Thu, 23 May 2024 16:20:19 -0400 Subject: [PATCH 17/17] fix(react-api-client): filter out unknown modules in the /modules endpoint. (#15256) --- api-client/src/modules/__fixtures__/index.ts | 23 +++++++++++++++++++ .../__tests__/useModulesQuery.test.tsx | 18 +++++++++++++++ .../src/modules/useModulesQuery.ts | 16 ++++++++++++- 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/api-client/src/modules/__fixtures__/index.ts b/api-client/src/modules/__fixtures__/index.ts index 52bb9dc57f0..bae11279f43 100644 --- a/api-client/src/modules/__fixtures__/index.ts +++ b/api-client/src/modules/__fixtures__/index.ts @@ -93,6 +93,29 @@ export const mockModulesResponse = [ }, ] +export const mockUnknownModuleResponse = [ + ...mockModulesResponse, + { + name: 'unknown', + displayName: 'UnknownModule', + moduleModel: 'unknownModule', + port: '/dev/unknown', + usbPort: { + port: 0, + hub: false, + portGroup: 'unknown', + path: '', + }, + serial: 'dummySerialMD', + model: 'unknown_v1.1', + revision: 'unknown_v1.1', + fwVersion: 'dummyVersionMD', + hasAvailableUpdate: false, + status: 'engaged', + data: {}, + }, +] + export const v2MockModulesResponse = [ { name: 'thermocycler', diff --git a/react-api-client/src/modules/__tests__/useModulesQuery.test.tsx b/react-api-client/src/modules/__tests__/useModulesQuery.test.tsx index c91e0517c28..5d0f3d9e238 100644 --- a/react-api-client/src/modules/__tests__/useModulesQuery.test.tsx +++ b/react-api-client/src/modules/__tests__/useModulesQuery.test.tsx @@ -5,6 +5,7 @@ import { renderHook, waitFor } from '@testing-library/react' import { getModules, mockModulesResponse, + mockUnknownModuleResponse, v2MockModulesResponse, } from '@opentrons/api-client' import { useHost } from '../../api' @@ -21,6 +22,10 @@ const MODULES_RESPONSE = { data: mockModulesResponse, meta: { totalLength: 0, cursor: 0 }, } +const UNKNOWN_MODULES_RESPONSE = { + data: mockUnknownModuleResponse, + meta: { totalLength: 0, cursor: 0 }, +} const V2_MODULES_RESPONSE = { data: v2MockModulesResponse } describe('useModulesQuery hook', () => { @@ -67,6 +72,19 @@ describe('useModulesQuery hook', () => { expect(result.current.data).toEqual(MODULES_RESPONSE) }) }) + it('should filter out unknown modules', async () => { + vi.mocked(useHost).mockReturnValue(HOST_CONFIG) + vi.mocked(getModules).mockResolvedValue({ + data: UNKNOWN_MODULES_RESPONSE, + } as Response) + + const { result } = renderHook(useModulesQuery, { wrapper }) + + await waitFor(() => { + expect(result.current.data).toEqual(MODULES_RESPONSE) + }) + }) + it('should return an empty array if an old version of modules returns', async () => { vi.mocked(useHost).mockReturnValue(HOST_CONFIG) vi.mocked(getModules).mockResolvedValue({ diff --git a/react-api-client/src/modules/useModulesQuery.ts b/react-api-client/src/modules/useModulesQuery.ts index 01b81e25aef..f4618e4f849 100644 --- a/react-api-client/src/modules/useModulesQuery.ts +++ b/react-api-client/src/modules/useModulesQuery.ts @@ -3,6 +3,7 @@ import { getModules } from '@opentrons/api-client' import { useHost } from '../api' import type { UseQueryResult, UseQueryOptions } from 'react-query' import type { HostConfig, Modules } from '@opentrons/api-client' +import { MODULE_MODELS } from '@opentrons/shared-data' export type UseModulesQueryOptions = UseQueryOptions @@ -26,7 +27,20 @@ export function useModulesQuery( } } }), - { enabled: host !== null, ...options } + { + enabled: host !== null, + // Filter unknown modules so we don't block the app, which + // can happen when developing new devices not yet known to the client. + select: resp => { + return { + ...resp, + data: resp.data.filter(module => + MODULE_MODELS.includes(module.moduleModel) + ), + } + }, + ...options, + } ) return query