-
Notifications
You must be signed in to change notification settings - Fork 178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(app, api-client): add utilities for parsing deck config via addressable areas #13947
Conversation
… protocol commands
…addressable areas Instead of explicit load fixture commands, extrapolate the simplest possible setup for the deck given the included addressable areas referenced in a set of protocol commands Closes RAUT-853
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## edge #13947 +/- ##
==========================================
- Coverage 70.82% 70.49% -0.34%
==========================================
Files 2504 2509 +5
Lines 70465 72211 +1746
Branches 8616 9226 +610
==========================================
+ Hits 49908 50906 +998
- Misses 18457 19057 +600
- Partials 2100 2248 +148
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@@ -91,11 +91,13 @@ export type LabwareLocation = | |||
| { slotName: string } | |||
| { moduleId: string } | |||
| { labwareId: string } | |||
| { addressableAreaName: string } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you need to add this here? Aren't all the components (supposed to be) using LabwareLocation
from schema v8?
OT2RobotMixin, | ||
OT3RobotMixin, | ||
ProtocolBase, | ||
ProtocolFile, | ||
} from '@opentrons/shared-data/protocol/types/schemaV8' | ||
import type { LoadLabwareCreateCommand } from '@opentrons/shared-data/protocol/types/schemaV7' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this being imported from schemaV7 now?
@@ -75,7 +75,10 @@ export const getLabwareRenderInfo = ( | |||
) | |||
} | |||
// TODO(bh, 2023-10-19): convert this to deck definition v4 addressableAreas |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this TODO be deleted now?
// TODO(BC, 11/6/23): once moveToAddressableArea command exists add it back here | ||
// else if (command.commandType === 'moveToAddressableArea') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moveToAddressableArea
command exists in types on edge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
functions look good to me - this will affect BaseDeck renders of fixture SVGs but i think we should merge this later today and resolve inconsistencies in #13931
export interface CutoutFixture { | ||
id: CutoutFixtureId | ||
mayMountTo: CutoutId[] | ||
displayName: string | ||
providesAddressableAreas: Record<CutoutId, AddressableAreaName[]> | ||
} | ||
|
||
export interface AddressableArea { | ||
id: AddressableAreaName | ||
areaType: 'slot' | 'movableTrash' | 'fixedTrash' | 'wasteChute' | ||
matingSurfaceUnitVector: UnitVectorTuple | ||
offsetFromCutoutFixture: UnitVectorTuple | ||
boundingBox: Dimensions | ||
displayName: string | ||
compatibleModuleTypes: ModuleType[] | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a note, some of this is duplicated in https://github.com/Opentrons/opentrons/pull/13931/files#diff-d6a873bb9d1ca073c74cc090513ae5c1f010fb305b1440f8607b7e4e34fcb002, some types more complete there, some more complete here. probably this will merge first so can be accounted for in that PR
| 'singleLeftSlot' | ||
| 'singleCenterSlot' | ||
| 'singleRightSlot' | ||
| 'stagingAreaRightSlot' | ||
| 'trashBinAdapter' | ||
| 'wasteChuteRightAdapterCovered' | ||
| 'wasteChuteRightAdapterNoCover' | ||
| 'stagingAreaSlotWithWasteChuteRightAdapterCovered' | ||
| 'stagingAreaSlotWithWasteChuteRightAdapterNoCover' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another note, BaseDeck will need these as string literal constants and sets of "singleSlotFixtures" at some point. and probably other components as well. might be better to adjust in #13931 with the understanding that the deck map won't render correctly until that PR is merged.
addressableArea: AddressableAreaName, | ||
cutoutFixtures: CutoutFixture[] | ||
): CutoutId | null { | ||
return cutoutFixtures.reduce<CutoutId | null>((acc, cutoutFixture) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this might read simpler as a forEach
Overview
Instead of explicit load fixture commands, extrapolate the simplest possible setup for the deck
given the included addressable areas referenced in a set of protocol commands
Closes RAUT-853
Changelog
addressableAreaName
toLabwareLocation
typeparseAllAddressableAreas
util for iterating through commands and scraping out addressableArea referencesgetSimplestDeckConfigForProtocolCommands
util that returns the simplest compatible set of cutout fixtures that would satisfy the specced addressable areasRisk assessment
low