From 13c5a02c35dda580bf4a900846b82120028b5d1b Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Thu, 24 Oct 2024 11:00:00 -0400 Subject: [PATCH] fix(app): remove some dangerous reverse()s There weer a couple command texts that were doing a `.reverse()` of the commands array. This is O(N) which is gross and also might be cached which would cause bugs, so replace it with some uses of findLast() that definitely weren't inspired by wanting to do some fun typing exercises. --- .../utils/getFinalLabwareLocation.ts | 50 +++++++++++---- .../utils/getWellRange.ts | 61 +++++++++++++------ 2 files changed, 80 insertions(+), 31 deletions(-) diff --git a/app/src/local-resources/commands/hooks/useCommandTextString/utils/getFinalLabwareLocation.ts b/app/src/local-resources/commands/hooks/useCommandTextString/utils/getFinalLabwareLocation.ts index 80cd4e26a4e..e674794a265 100644 --- a/app/src/local-resources/commands/hooks/useCommandTextString/utils/getFinalLabwareLocation.ts +++ b/app/src/local-resources/commands/hooks/useCommandTextString/utils/getFinalLabwareLocation.ts @@ -1,4 +1,25 @@ -import type { LabwareLocation, RunTimeCommand } from '@opentrons/shared-data' +import type { + LabwareLocation, + RunTimeCommand, + LoadLabwareRunTimeCommand, + MoveLabwareRunTimeCommand, +} from '@opentrons/shared-data' + +const findLastAt = ( + arr: readonly T[], + pred: ((el: T) => boolean) | ((el: T) => el is U) +): [U, number] | [undefined, -1] => { + let arrayLoc = -1 + const lastEl = arr.findLast((el: T, idx: number): el is U => { + arrayLoc = idx + return pred(el) + }) + if (lastEl === undefined) { + return [undefined, -1] + } else { + return [lastEl, arrayLoc] + } +} /** * given a list of commands and a labwareId, calculate the resulting location @@ -11,15 +32,22 @@ export function getFinalLabwareLocation( labwareId: string, commands: RunTimeCommand[] ): LabwareLocation | null { - for (const c of commands.reverse()) { - if (c.commandType === 'loadLabware' && c.result?.labwareId === labwareId) { - return c.params.location - } else if ( - c.commandType === 'moveLabware' && - c.params.labwareId === labwareId - ) { - return c.params.newLocation - } + const [lastMove, lastMoveIndex] = findLastAt( + commands, + (c: RunTimeCommand): c is MoveLabwareRunTimeCommand => + c.commandType === 'moveLabware' && c.params.labwareId === labwareId + ) + + const [lastLoad, lastLoadIndex] = findLastAt( + commands, + (c: RunTimeCommand): c is LoadLabwareRunTimeCommand => + c.commandType === 'loadLabware' && c.result?.labwareId === labwareId + ) + if (lastMoveIndex > lastLoadIndex) { + return lastMove?.params?.newLocation ?? null + } else if (lastLoadIndex > lastMoveIndex) { + return lastLoad?.params?.location ?? null + } else { + return null } - return null } diff --git a/app/src/local-resources/commands/hooks/useCommandTextString/utils/getWellRange.ts b/app/src/local-resources/commands/hooks/useCommandTextString/utils/getWellRange.ts index a0700357413..791ddf2f4c3 100644 --- a/app/src/local-resources/commands/hooks/useCommandTextString/utils/getWellRange.ts +++ b/app/src/local-resources/commands/hooks/useCommandTextString/utils/getWellRange.ts @@ -1,5 +1,42 @@ import { getPipetteNameSpecs } from '@opentrons/shared-data' -import type { PipetteName, RunTimeCommand } from '@opentrons/shared-data' +import type { + PipetteName, + RunTimeCommand, + ConfigureNozzleLayoutRunTimeCommand, +} from '@opentrons/shared-data' + +const usedChannelsFromCommand = ( + command: ConfigureNozzleLayoutRunTimeCommand | undefined, + defaultChannels: number +): number => + command?.params?.configurationParams?.style === 'SINGLE' + ? 1 + : command?.params?.configurationParams?.style === 'COLUMN' + ? 8 + : defaultChannels + +const usedChannelsForPipette = ( + pipetteId: string, + commands: RunTimeCommand[], + defaultChannels: number +): number => + usedChannelsFromCommand( + commands.findLast( + (c: RunTimeCommand): c is ConfigureNozzleLayoutRunTimeCommand => + c.commandType === 'configureNozzleLayout' && + c.params?.pipetteId === pipetteId + ), + defaultChannels + ) + +const usedChannels = ( + pipetteId: string, + commands: RunTimeCommand[], + pipetteChannels: number +): number => + pipetteChannels === 96 + ? usedChannelsForPipette(pipetteId, commands, pipetteChannels) + : pipetteChannels /** * @param pipetteName name of pipette being used @@ -16,26 +53,10 @@ export function getWellRange( const pipetteChannels = pipetteName ? getPipetteNameSpecs(pipetteName)?.channels ?? 1 : 1 - let usedChannels = pipetteChannels - if (pipetteChannels === 96) { - for (const c of commands.reverse()) { - if ( - c.commandType === 'configureNozzleLayout' && - c.params?.pipetteId === pipetteId - ) { - // TODO(sb, 11/9/23): add support for quadrant and row configurations when needed - if (c.params.configurationParams.style === 'SINGLE') { - usedChannels = 1 - } else if (c.params.configurationParams.style === 'COLUMN') { - usedChannels = 8 - } - break - } - } - } - if (usedChannels === 96) { + const channelCount = usedChannels(pipetteId, commands, pipetteChannels) + if (channelCount === 96) { return 'A1 - H12' - } else if (usedChannels === 8) { + } else if (channelCount === 8) { const column = wellName.substr(1) return `A${column} - H${column}` }