feat(protocol-designer): add designerApplication labware, pipettes, m… #17432

jerader · 2025-02-05T16:42:12Z

…odules keys

Overview

This PR introduces designerApplication top level keys for modules, labware, and pipettes. So exporting a JSON protocol should look like this:

pipettes: {
   "0b3f2210-75c7-11ea-b42f-4b64e50f43e5": {
     "name": "p300_single_gen2"
   }
 },
 modules: {
   "0b419310-75c7-11ea-b42f-4b64e50f43e5:magneticModuleType": {
     model: "magneticModuleV2"
   },
 },
 labware: {
   "0b44c760-75c7-11ea-b42f-4b64e50f43e5:opentrons/opentrons_96_tiprack_300ul/1": {
     displayName: "A nickname",
     labwareDefURI: "opentrons/opentrons_96_tiprack_300ul/1"
   },
 }

then when you import a protocol, the migration will grab that info from the load commands and the load-file reducer will use that info to populate the labwareEntities, pipetteEntities, and moduleEntities.

Additionally, this pr deprecates the defaultValues key that was not in use at all

Test Plan and Hands on Testing

Create a protocol and export it. the JSON file should include these new keys

upload an old protocol and see that it migrates correctly

Changelog

fix the types and add these new keys onto designerApplication type
add to the 8_5_0 migration, fix up the load-file reducer to grab this new information
add some test coverage for the utils

Risk assessment

med-ish,

codecov · 2025-02-05T16:48:22Z

Codecov Report

Attention: Patch coverage is 72.78912% with 40 lines in your changes missing coverage. Please review.

Project coverage is 18.19%. Comparing base (9d28971) to head (a66ed78).
Report is 2 commits behind head on edge.

Files with missing lines	Patch %	Lines
protocol-designer/src/step-forms/reducers/index.ts	0.00%	22 Missing ⚠️
...igration/utils/getEquipmentLoadInfoFromCommands.ts	84.81%	12 Missing ⚠️
protocol-designer/src/load-file/migration/8_5_0.ts	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             edge   #17432      +/-   ##
==========================================
+ Coverage   18.15%   18.19%   +0.03%     
==========================================
  Files        3173     3174       +1     
  Lines      229557   229629      +72     
  Branches     6889     6908      +19     
==========================================
+ Hits        41682    41781      +99     
+ Misses     187875   187848      -27

Flag	Coverage Δ
protocol-designer	`17.38% <72.78%> (+0.03%)`	⬆️

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

Files with missing lines	Coverage Δ
...ol-designer/src/file-data/selectors/fileCreator.ts	`88.08% <100.00%> (-0.19%)`	⬇️
protocol-designer/src/file-data/selectors/utils.ts	`78.91% <100.00%> (+4.91%)`	⬆️
protocol-designer/src/file-types.ts	`71.42% <ø> (ø)`
protocol-designer/src/load-file/migration/8_5_0.ts	`1.23% <0.00%> (-0.04%)`	⬇️
...igration/utils/getEquipmentLoadInfoFromCommands.ts	`84.81% <84.81%> (ø)`
protocol-designer/src/step-forms/reducers/index.ts	`58.31% <0.00%> (+2.18%)`	⬆️

jerader · 2025-02-05T16:58:10Z

protocol-designer/src/file-data/selectors/fileCreator.ts

@@ -268,7 +247,7 @@ export const createFile: Selector<ProtocolFile> = createSelector(
      commandAnnotations,
    }

-    const protocolBase: ProtocolBase<DesignerApplicationDataV8_5> = {
+    const protocolBase: ProtocolBase<PDMetadata> = {


realized that PDMetadata is the proper type to use here - we had several designerApplicationData types floating around.

jerader · 2025-02-05T16:59:13Z

protocol-designer/src/file-types.ts

-// NOTE: PD currently supports saving both v3 and v4, depending on whether it has modules
-export type PDProtocolFile =
-  | ProtocolFileV3<PDMetadata>
-  | ProtocolFileV4<PDMetadata>
-  | ProtocolFileV5<PDMetadata>
-  | ProtocolFileV6<PDMetadata>


this was significantly outdated... still referenced protocol schema v6

protocol-designer/src/step-forms/reducers/index.ts

…odules keys closes AUTH-1407

ddcc4 · 2025-02-05T19:34:47Z

protocol-designer/fixtures/protocol/8/doItAllV3MigratedToV8.json

-        "dispense_mmFromBottom": 1,
-        "touchTip_mmFromTop": -1,
-        "blowout_mmFromTop": 0
-      },


ddcc4 · 2025-02-05T19:49:02Z

protocol-designer/src/load-file/migration/utils/getEquipmentLoadInfoFromCommands.ts

+        )
+      }
+      const id = labwareId ?? 'unknown id'
+      const displayName = labwareDefinitions[labwareDefURI].metadata.displayName


Is the if (labwareDefinitions[labwareDefURI] == null) supposed to guard against something? Wouldn't this line of code still crash if labwareDefinitions[labwareDefURI] is in fact null?

i guess its to help debug. it shouldn't be null, but if it is, we can see the console errors and know how to fix it faster 😄

Oh, I mean like: if you expect the code not to be runnable if labwareDefinitions[labwareDefURI] == null, wouldn't it be better to just throw an exception rather than console.error and letting the code continue to run?

In the #protocol-designer channel, we often get user reports like "An unknown error has occurred. Contact support with the following message: Cannot read properties of undefined." Our users don't have the Chrome Inspector open, so they won't see any messages in console.log, right? If you throw the error message as an exception, at least we'll get better error reports from users :)

ddcc4 · 2025-02-05T19:50:39Z

protocol-designer/src/load-file/migration/utils/getEquipmentLoadInfoFromCommands.ts

+
+      return {
+        ...acc,
+        [id]: {


Hm, just in case labwareId is null, would it be better to pick an id that's more unique than 'unknown id', so that all the missing labwareIds don't get smushed together into the same entry here?

so, according to load_labware.py, labwareId is optional. However, PD will always populate labwareId in the loadLabware JSON commands. We shouldn't run into a case where this is null ever unless the user corrupted their protocol.

but similarly to the above comment about labwareDefURI being null in labwareDefinitions, i added a console.error to help debug in the event that labwareId is null.

#17432) …odules keys closes AUTH-1407 This PR introduces `designerApplication` top level keys for `modules`, `labware`, and `pipettes`. Then when you import a protocol, the migration will grab that info from the load commands and the `load-file` reducer will use that info to populate the `labwareEntities`, `pipetteEntities`, and `moduleEntities`. Additionally, this pr deprecates the `defaultValues` key that was not in use at all

jerader commented Feb 5, 2025

View reviewed changes

protocol-designer/src/step-forms/reducers/index.ts Outdated Show resolved Hide resolved

jerader marked this pull request as ready for review February 5, 2025 17:34

jerader requested a review from a team as a code owner February 5, 2025 17:34

jerader requested a review from ddcc4 February 5, 2025 17:34

jerader added 2 commits February 5, 2025 14:39

feat(protocol-designer): add designerApplication labware, pipettes, m…

0fee111

…odules keys closes AUTH-1407

fix migration tests

a66ed78

jerader force-pushed the pd_lw-mod-pip-da branch from facc82c to a66ed78 Compare February 5, 2025 19:40

ddcc4 approved these changes Feb 5, 2025

View reviewed changes

jerader merged commit f16f6a0 into edge Feb 5, 2025
34 checks passed

jerader deleted the pd_lw-mod-pip-da branch February 5, 2025 20:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(protocol-designer): add designerApplication labware, pipettes, m… #17432

feat(protocol-designer): add designerApplication labware, pipettes, m… #17432

jerader commented Feb 5, 2025

codecov bot commented Feb 5, 2025 •

edited

Loading

jerader Feb 5, 2025

jerader Feb 5, 2025

ddcc4 Feb 5, 2025

ddcc4 Feb 5, 2025

jerader Feb 5, 2025

ddcc4 Feb 5, 2025

ddcc4 Feb 5, 2025

jerader Feb 5, 2025 •

edited

Loading

jerader Feb 5, 2025

feat(protocol-designer): add designerApplication labware, pipettes, m… #17432

feat(protocol-designer): add designerApplication labware, pipettes, m… #17432

Conversation

jerader commented Feb 5, 2025

Overview

Test Plan and Hands on Testing

Changelog

Risk assessment

codecov bot commented Feb 5, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerader Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Feb 5, 2025 •

edited

Loading

jerader Feb 5, 2025 •

edited

Loading