feat(ot3): add shared data support for ot3 pipettes #11704

Laura-Danielle · 2022-11-09T19:32:02Z

Overview

We need a better way to encapsulate the new functionality that ot3 pipettes has, as well as the
types of pipettes available. I will be splitting up this work into a series of bits of work ticketed here.

Changelog

Bump the schema version on the pipettes (for now only GEN 3s are supported)
Split out the pipette schemas into more "reasonable" sections that are also easier to use

Review requests

Can someone make sure my js tests are all actually running correctly?
Anything else you think is missing from the schemas?
Anything you don't like about the schemas as they are set-up now?

Risk assessment

Low. Not being used yet.

We need a better way to encapsulate the new functionality that ot3 pipettes has, as well as the types of pipettes available.

codecov · 2022-11-09T19:33:20Z

Codecov Report

❗ No coverage uploaded for pull request base (RLIQ-131-support-96-channel@934c322). Click here to learn what that means.
The diff coverage is n/a.

@@                      Coverage Diff                       @@
##             RLIQ-131-support-96-channel   #11704   +/-   ##
==============================================================
  Coverage                               ?   74.40%           
==============================================================
  Files                                  ?     2089           
  Lines                                  ?    58016           
  Branches                               ?     6158           
==============================================================
  Hits                                   ?    43164           
  Misses                                 ?    13403           
  Partials                               ?     1449

Flag	Coverage Δ
app	`72.61% <0.00%> (?)`
notify-server	`88.26% <0.00%> (?)`
protocol-designer	`45.86% <0.00%> (?)`
shared-data	`84.87% <0.00%> (?)`
step-generation	`88.46% <0.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

sfoster1

This is so much nicer!

That said,

let's get the new schema data in the README
can you say a little more in the README and/or pr description about what the 3d files are for?
can you say a little more in the README and/or pr description about what the T200/T50 things are for and how they'll be used?

sfoster1 · 2022-11-09T21:42:25Z

shared-data/pipette/definitions/2/geometry/eight_channel/p50/1.json

@@ -0,0 +1,5 @@
+{
+  "$otSharedSchema": "#/pipette/schemas/2/pipetteGeometrySchema.json",
+  "pathTo3D": "pipette/definitions/2/eight_channel/p1000/placeholder.gltf",


koji · 2022-11-10T21:18:09Z

shared-data/js/__tests__/pipetteSchemaV2.test.ts

+  })
+
+  liquidPaths.forEach(liquidPath => {
+    const filename = path.parse(liquidPath).base


As CI says, you don't use filename. Maybe just remove this line?

andySigler

Went through and checked config values based off values we're currently using in testing

shared-data/pipette/definitions/2/liquid/single_channel/p50/1.json

shared-data/pipette/definitions/2/liquid/single_channel/p1000/1.json

shared-data/pipette/definitions/2/liquid/ninety_six_channel/p1000/1.json

shared-data/pipette/definitions/2/general/ninety_six_channel/p1000/1.json

mcous

I've got a couple tiny comments with an eye towards RCORE-382 and some yet-to-be-written PAPI tickets for 96-channel support, but otherwise this looks really solid and a vast improvement over the old configs

mcous · 2022-11-15T16:12:32Z

shared-data/pipette/schemas/2/pipetteLiquidPropertiesSchema.json

+    "patternProperties": {
+      "description": "Tip specific liquid handling properties for a given pipette. Using the active tip on a pipette, we will look up the pipetting configurations associated with that tip+pipette combo.",
+      "type": "object",
+      "$comment": "Example key: 'T50'",
+      "^T[0-9]{2,4}": {


A couple notes:

Pattern properties are not easy to to map into Pedantic and other runtime models

I would softly prefer lowercase identifiers to upper or mixed-case

Could this be a list instead with a string field for the t50 (or whatever) portion?

@mcous could I wrap the tip types in a top level key instead? I.e.:

supportedTips: { t50 {...}, t200 {...}, }

etc? That way I can load the pydantic models based on the guaranteed keys inside each nested "tip" object and then I can use the top level pattern property keys to create a look-up table.

mcous · 2022-11-15T16:16:42Z

shared-data/pipette/schemas/2/pipettePropertiesSchema.json

+    "availableSensors": {
+      "type": "object",
+      "description": "object with keyed by sensor and number available",
+      "required": ["pressure", "capacitive", "environment"],
+      "properties": {
+        "pressure": { "enum": [1, 2] },
+        "capacitive": { "enum": [1, 2] },
+        "environment": { "enum": [1] }
+      }
+    },


This specific capabilities list is a little hard to understand coming in blind. Can we make one of both of these changes?

Change to a list so we can define new sensors without changing the schema?

Do something like pressure: { count: number } rather than pressure: number to make it more obvious what's going on when reading the data?

As an aside, how is the sensor count data used?

It is not currently used, but it will be used in the expanded pipette object class for ot3 pipettes to know how many sensor objects to build (will mainly be used for liquid sensing purposes)

@mcous what do you think about this:

Available sensors is still an object that contains:

A list of available sensors (can be anything)

A pattern property which is keyed by sensor type and the object it contains is {count: number}

Then, an available sensors object might look like:

availableSensors: { "sensors": ["pressure", "capacitive", "environment"], "pressure": {count: 2}, "capacitive": {count: 1}, "environment": {count: 1} }

talked f2f, seth is OK with merging into feature branch

sfoster1

Looks awesome!

Laura-Danielle added 15 commits November 8, 2022 11:01

feat(ot3): add shared data support for ot3 pipettes

fe1340e

We need a better way to encapsulate the new functionality that ot3 pipettes has, as well as the types of pipettes available.

feat(ot3): Add skeletons of information for all new pipettes

c6baf76

feat(ot3): Add pipette config for ot3 pipettes

252d7d9

Add data types for pipette schema

f44f8ca

update pipette data to match hardware

7580d3c

add data types to handle json files

318b576

Handle ot3 pipette objects

6a7865d

wrap current pipette schemas in version 1

77fcd33

feat(shared-data): add liquid handling for multiple tips

cbbe41b

update geometry for low throughput pipettes

2f2a0f9

feat: update geometry for high throughput pipettes

85735ad

json formatting

eb58d68

fix js tests to use version 1 of pipette schemas

6271fe6

add new test for version two of pipette schemas

d66c066

get rid of unused files

7324349

Laura-Danielle requested review from a team and mcous and removed request for a team November 9, 2022 19:32

Don't include files from other branch

3547105

Laura-Danielle requested a review from a team as a code owner November 9, 2022 19:42

sfoster1 previously requested changes Nov 9, 2022

View reviewed changes

koji reviewed Nov 10, 2022

View reviewed changes

andySigler requested changes Nov 11, 2022

View reviewed changes

Laura-Danielle added 4 commits November 14, 2022 17:59

update configurations to match current testing values

06e306f

fix js check

9176ed0

update readme

561157f

formatting

917e0a2

Laura-Danielle requested a review from koji November 15, 2022 15:15

Laura-Danielle requested review from andySigler and sfoster1 November 15, 2022 15:15

mcous reviewed Nov 15, 2022

View reviewed changes

andySigler approved these changes Nov 15, 2022

View reviewed changes

Laura-Danielle added 3 commits November 15, 2022 13:32

make lower case keys

83f5159

add supportedtips top level key

ee07df5

Modify available sensors object

a036f7d

sfoster1 approved these changes Nov 16, 2022

View reviewed changes

Laura-Danielle merged commit 355ee9c into RLIQ-131-support-96-channel Nov 16, 2022

Laura-Danielle deleted the shared-data_ot3-pipette-support branch November 16, 2022 16:16

Laura-Danielle added a commit that referenced this pull request Nov 29, 2022

feat(ot3): add shared data support for ot3 pipettes (#11704)

352fdfd

Laura-Danielle added a commit that referenced this pull request Dec 16, 2022

feat(ot3): add shared data support for ot3 pipettes (#11704)

20ac5e4

Laura-Danielle added a commit that referenced this pull request Dec 27, 2022

feat(ot3): add shared data support for ot3 pipettes (#11704)

deebcc8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ot3): add shared data support for ot3 pipettes #11704

feat(ot3): add shared data support for ot3 pipettes #11704

Laura-Danielle commented Nov 9, 2022

codecov bot commented Nov 9, 2022 •

edited

Loading

sfoster1 left a comment

sfoster1 Nov 9, 2022

koji Nov 10, 2022

andySigler left a comment

mcous left a comment

mcous Nov 15, 2022

Laura-Danielle Nov 15, 2022

mcous Nov 15, 2022

Laura-Danielle Nov 15, 2022

Laura-Danielle Nov 15, 2022 •

edited

Loading

sfoster1 left a comment

feat(ot3): add shared data support for ot3 pipettes #11704

feat(ot3): add shared data support for ot3 pipettes #11704

Conversation

Laura-Danielle commented Nov 9, 2022

Overview

Changelog

Review requests

Risk assessment

codecov bot commented Nov 9, 2022 • edited Loading

Codecov Report

sfoster1 left a comment

Choose a reason for hiding this comment

sfoster1 Nov 9, 2022

Choose a reason for hiding this comment

koji Nov 10, 2022

Choose a reason for hiding this comment

andySigler left a comment

Choose a reason for hiding this comment

mcous left a comment

Choose a reason for hiding this comment

mcous Nov 15, 2022

Choose a reason for hiding this comment

Laura-Danielle Nov 15, 2022

Choose a reason for hiding this comment

mcous Nov 15, 2022

Choose a reason for hiding this comment

Laura-Danielle Nov 15, 2022

Choose a reason for hiding this comment

Laura-Danielle Nov 15, 2022 • edited Loading

Choose a reason for hiding this comment

sfoster1 left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 9, 2022 •

edited

Loading

Laura-Danielle Nov 15, 2022 •

edited

Loading