Skip to content
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.

Sign up for GitHub

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

feat(shared-data): correct existing labware defs engageHeight #5261

Merged
merged 3 commits into from
Mar 23, 2020
Merged

feat(shared-data): correct existing labware defs engageHeight #5261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
135fe2f
feat(shared-data): correct existing labware defs engageHeight
IanLondon Mar 20, 2020
fe01fb5
fix namespace check, refactor
IanLondon Mar 20, 2020
d5f19ce
fix error where short vs long conversion was inverted
IanLondon Mar 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions protocol-designer/src/ui/modules/selectors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
MAGNETIC_MODULE_TYPE,
TEMPERATURE_MODULE_TYPE,
THERMOCYCLER_MODULE_TYPE,
MAGNETIC_MODULE_V1,
} from '@opentrons/shared-data'
import mapValues from 'lodash/mapValues'
import { selectors as stepFormSelectors } from '../../step-forms'
Expand Down Expand Up @@ -139,9 +140,19 @@ export const getMagnetLabwareEngageHeight: Selector<
stepFormSelectors.getInitialDeckSetup,
getSingleMagneticModuleId,
(initialDeckSetup, magnetModuleId) => {
const labware =
magnetModuleId && getLabwareOnModule(initialDeckSetup, magnetModuleId)
return (labware && getLabwareDefaultEngageHeight(labware.def)) || null
if (magnetModuleId == null) return null

const moduleModel = initialDeckSetup.modules[magnetModuleId]?.model
const labware = getLabwareOnModule(initialDeckSetup, magnetModuleId)
const engageHeightMm = labware
? getLabwareDefaultEngageHeight(labware.def)
: null

if (engageHeightMm != null && moduleModel === MAGNETIC_MODULE_V1) {
// convert to 'short mm' units for GEN1
return engageHeightMm * 2
}
return engageHeightMm
}
)

Expand Down
3 changes: 0 additions & 3 deletions shared-data/js/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ export const MODULE_MODELS: Array<ModuleModel> = [
...THERMOCYCLER_MODULE_MODELS,
]

// offset added to parameters.agneticModuleEngageHeight for displaying reccomended height in PD
export const ENGAGE_HEIGHT_OFFSET = -4
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed this from constants b/c nobody aside from getLabwareDefaultEngageHeight should use it, as far as I can figure. I think wanting the -4 would be a sign you're not going through the getter and probably you should use the getter...?


export const MODULE_TYPES = [
TEMPERATURE_MODULE_TYPE,
MAGNETIC_MODULE_TYPE,
Expand Down
33 changes: 30 additions & 3 deletions shared-data/js/getLabware.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import mapValues from 'lodash/mapValues'
// TODO: Ian 2019-06-04 remove the shared-data build process for labware v1
import definitions from '../build/labware.json'
import {
SLOT_RENDER_HEIGHT,
FIXED_TRASH_RENDER_HEIGHT,
ENGAGE_HEIGHT_OFFSET,
OPENTRONS_LABWARE_NAMESPACE,
SLOT_RENDER_HEIGHT,
} from './constants'
import type {
LabwareDefinition1,
Expand Down Expand Up @@ -48,12 +48,39 @@ export function getIsTiprack(labwareDef: LabwareDefinition2): boolean {
return labwareDef.parameters.isTiprack
}

// NOTE: these labware definitions in _SHORT_MM_LABWARE_DEF_LOADNAMES
// were written in "short mm" = 0.5mm, but
// we will write all future definitions in actual mm.
// These whitelisted labware also have engage heights measured from home switch
// instead of from labware bottom, which is why we add ENGAGE_HEIGHT_OFFSET.
//
// Ideally instead of using this whitelist, we would publish a new version
// of these definitions with corrected labware heights. However, we don't
// support labware versioning well enough yet.
const _SHORT_MM_LABWARE_DEF_LOADNAMES = [
'biorad_96_wellplate_200ul_pcr',
'nest_96_wellplate_100ul_pcr_full_skirt',
'usascientific_96_wellplate_2.4ml_deep',
]
// offset added to parameters.magneticModuleEngageHeight to convert older labware
// definitions from "distance from home switch" to "distance from labware bottom"
// Note: this is in actual mm, not "short mm" :)
const ENGAGE_HEIGHT_OFFSET = -4

export function getLabwareDefaultEngageHeight(
labwareDef: LabwareDefinition2
): number | null {
const rawEngageHeight: ?number =
labwareDef.parameters.magneticModuleEngageHeight
return rawEngageHeight == null ? null : rawEngageHeight + ENGAGE_HEIGHT_OFFSET
if (
labwareDef.namespace === OPENTRONS_LABWARE_NAMESPACE &&
_SHORT_MM_LABWARE_DEF_LOADNAMES.includes(labwareDef.parameters.loadName)
) {
return rawEngageHeight == null
? null
: rawEngageHeight / 2 + ENGAGE_HEIGHT_OFFSET
}
return rawEngageHeight == null ? null : rawEngageHeight
}

/* Render Helpers */
Expand Down