feat(step-generation, protocol-designer): add timeline error for off deck labware #12997

jerader · 2023-06-27T20:16:14Z

closes RAUT-525

Overview

If you try to interact with a labware that you moved off deck previously in the protocol, you will get a timeline error

Also fix up some weirdness in the Select component causing some dropdowns to have too wide a height and width.

Test Plan

move the labware off deck and then add an aspirate, dispense, blow out, moveToWell with the labware that is off deck as a source. When you create the command, you should see an error
try to move the labware back onto the deck after it is currently off deck with the gripper, you should see the same error

Changelog

add i18n strings back for HEATER_SHAKER_LATCH_CLOSED - i think i had a merge conflict and accidentally deleted the previous i18n strings 😢
create errorCreator labwareOffDeck and add to the appropriate atomic commands
add the error to i18n
change the width = 100% to be in a parent component rather than tweaking the Select component that more components use.

Review requests

see test plan

Risk assessment

low

codecov · 2023-06-27T20:24:06Z

Codecov Report

Merging #12997 (bf1d1aa) into edge (dd8efa6) will decrease coverage by 0.15%.
The diff coverage is 31.81%.

@@            Coverage Diff             @@
##             edge   #12997      +/-   ##
==========================================
- Coverage   72.79%   72.65%   -0.15%     
==========================================
  Files        2363     2368       +5     
  Lines       64585    64970     +385     
  Branches     7222     7374     +152     
==========================================
+ Hits        47016    47201     +185     
- Misses      15885    16055     +170     
- Partials     1684     1714      +30

Flag	Coverage Δ
app	`71.30% <ø> (-0.14%)`	⬇️
components	`65.47% <ø> (-1.28%)`	⬇️
labware-library	`49.61% <ø> (ø)`
protocol-designer	`44.00% <75.00%> (-0.32%)`	⬇️
step-generation	`84.28% <7.14%> (-0.85%)`	⬇️

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

Impacted Files	Coverage Δ
components/src/forms/Select.tsx	`73.33% <ø> (-5.24%)`	⬇️
...-generation/src/commandCreators/atomic/aspirate.ts	`88.37% <0.00%> (-4.32%)`	⬇️
...p-generation/src/commandCreators/atomic/blowout.ts	`88.23% <0.00%> (-11.77%)`	⬇️
...-generation/src/commandCreators/atomic/dispense.ts	`81.08% <0.00%> (-4.64%)`	⬇️
...neration/src/commandCreators/atomic/moveLabware.ts	`2.63% <0.00%> (-0.31%)`	⬇️
...eneration/src/commandCreators/atomic/moveToWell.ts	`88.09% <0.00%> (-4.41%)`	⬇️
step-generation/src/types.ts	`100.00% <ø> (ø)`
step-generation/src/errorCreators.ts	`87.17% <50.00%> (-2.01%)`	⬇️
...ponents/modals/FilePipettesModal/PipetteFields.tsx	`84.00% <75.00%> (-0.62%)`	⬇️

... and 27 files with indirect coverage changes

jerader · 2023-06-28T12:35:03Z

protocol-designer/src/components/modals/FilePipettesModal/FilePipettesModal.css

@@ -133,3 +133,7 @@
 .module_model {
  height: 3rem;
 }
+
+.pipette_select {


classNames was already being prop drilled so it made more sense to just add on to that rather than convert all the PipetteSelect components to using styled-components

b-cooper · 2023-06-28T15:15:00Z

protocol-designer/src/components/modals/FilePipettesModal/PipetteFields.tsx

@@ -99,24 +100,27 @@ export function PipetteFields(props: Props): JSX.Element {
    const nameBlockList = [...OT2_PIPETTES, 'p1000_96']


nit: if this is going to be called nameBlockList as a const, it would make more sense to have the ternary on robot type in this line. Right now, it is only actually the nameBlockList for Flexes.
e.g.

const nameBlockList = robotType === OT2_ROBOT_TYPE ? OT3_PIPETTES : [...OT2_PIPETTES, 'p1000_96']

oops ya, good call

b-cooper · 2023-06-28T15:18:06Z

step-generation/src/commandCreators/atomic/moveToWell.ts

should we also throw an error in moveLabware if the labware is source or dest is 'offDeck', and we're using the gripper?

yeah! that makes sense to me

i think the same error creator should work for this?

b-cooper · 2023-06-28T17:31:17Z

step-generation/src/commandCreators/atomic/moveLabware.ts

@@ -28,6 +28,8 @@ export const moveLabware: CommandCreator<MoveLabwareArgs> = (
        labware,
      })
    )
+  } else if (prevRobotState.labware[labware].slot === 'offDeck' && useGripper) {


this is only checking if the source location is off deck, we should also check if the destination is off deck here

okay just added it, however, the offDeck option is being filtered out in the labware location dropdown if a gripper is being used for it so hopefully users never run into this.

Cool thanks for updating it. Since step-generation and protocol-designer are two separate modules, we shouldn't rely on form or field level protections in PD to prevent accessing non-sensical parameters in step-generation. Also theoretically, step-generation could be used by more than just PD. Its job is to validate step args against all known errors and turn them into protocol engine commands.

jerader · 2023-06-29T12:54:28Z

protocol-designer/src/localization/en/alert.json

+      },
+      "LABWARE_OFF_DECK": {
+        "title": "Robot interacting with an off deck labware",
+        "body": "Labware must be on the deck for the robot to interact with it. To resolve this error, please make sure the labware is on deck."
+      },
+      "HEATER_SHAKER_LATCH_CLOSED": {
+        "title": "Heater-Shaker labware latch is closed",
+        "body": "Before the robot can interact with labware on the Heater-Shaker module, the labware latch must be open. To resolve this error, please add a Heater-Shaker step ahead of the current step, and set the labware latch status to \"open\"."


Note: going to have Ed review and rewrite these and include the change in a different PR.

Also, the previous i18n strings for HEATER_SHAKER_LATCH_CLOSED accidentally got deleted when i was rebasing something at some point, that is why it is randomly added here.

shlokamin

lgtm!

shlokamin · 2023-06-29T15:12:16Z

components/src/forms/Select.tsx

+import { Box } from '../primitives'
+import { SPACING } from '../ui-style-constants'


these should go before type imports 😛

jerader changed the title ~~feat(protocol-designer): add timeline error for off deck labware~~ feat(step-generation): add timeline error for off deck labware Jun 27, 2023

create labwareOffDeck errorCreator and add to atomic commands

67d3808

jerader force-pushed the pd_error-creator-for-off-deck branch from 9d309e6 to 67d3808 Compare June 28, 2023 11:50

jerader added 2 commits June 28, 2023 08:32

fix logic to fix tests

165be42

fix unnecessary change in select

b6a85b5

jerader commented Jun 28, 2023

View reviewed changes

jerader marked this pull request as ready for review June 28, 2023 12:35

jerader requested review from a team as code owners June 28, 2023 12:35

jerader requested review from shlokamin and b-cooper and removed request for a team June 28, 2023 12:35

jerader changed the title ~~feat(step-generation): add timeline error for off deck labware~~ feat(step-generation, protocol-designer): add timeline error for off deck labware Jun 28, 2023

jerader removed the request for review from a team June 28, 2023 12:36

b-cooper reviewed Jun 28, 2023

View reviewed changes

add error to moveLabware and fix PipetteFields const

78a21bf

b-cooper reviewed Jun 28, 2023

View reviewed changes

jerader added 2 commits June 28, 2023 13:50

add error when gripper is trying to move labware off deck

2834246

add back i18n keys that were deleted during rebasing

8665124

jerader commented Jun 29, 2023

View reviewed changes

shlokamin approved these changes Jun 29, 2023

View reviewed changes

fix import order

bf1d1aa

jerader merged commit 4142bdd into edge Jun 29, 2023

jerader deleted the pd_error-creator-for-off-deck branch June 29, 2023 16:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(step-generation, protocol-designer): add timeline error for off deck labware #12997

feat(step-generation, protocol-designer): add timeline error for off deck labware #12997

jerader commented Jun 27, 2023 •

edited

Loading

codecov bot commented Jun 27, 2023 •

edited

Loading

jerader Jun 28, 2023 •

edited

Loading

b-cooper Jun 28, 2023

jerader Jun 28, 2023

b-cooper Jun 28, 2023

jerader Jun 28, 2023

jerader Jun 28, 2023

b-cooper Jun 28, 2023

jerader Jun 28, 2023 •

edited

Loading

b-cooper Jun 28, 2023

jerader Jun 29, 2023 •

edited

Loading

shlokamin left a comment

shlokamin Jun 29, 2023

jerader Jun 29, 2023

		@@ -99,24 +100,27 @@ export function PipetteFields(props: Props): JSX.Element {
		const nameBlockList = [...OT2_PIPETTES, 'p1000_96']

		import { Box } from '../primitives'
		import { SPACING } from '../ui-style-constants'

feat(step-generation, protocol-designer): add timeline error for off deck labware #12997

feat(step-generation, protocol-designer): add timeline error for off deck labware #12997

Conversation

jerader commented Jun 27, 2023 • edited Loading

Overview

Test Plan

Changelog

Review requests

Risk assessment

codecov bot commented Jun 27, 2023 • edited Loading

Codecov Report

jerader Jun 28, 2023 • edited Loading

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 Jun 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerader Jun 29, 2023 • edited Loading

Choose a reason for hiding this comment

shlokamin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerader commented Jun 27, 2023 •

edited

Loading

codecov bot commented Jun 27, 2023 •

edited

Loading

jerader Jun 28, 2023 •

edited

Loading

jerader Jun 28, 2023 •

edited

Loading

jerader Jun 29, 2023 •

edited

Loading